[Proposal] Cleaning our POM deps by explicitly listing used dependencies?

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

[Proposal] Cleaning our POM deps by explicitly listing used dependencies?

vmassol
Administrator
Hi devs,

Running mvn dependency:dependency-analyze produces interesting results.

For example:

[INFO] ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
[INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
[INFO] ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------

[INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @ xwiki-commons-properties ---
[WARNING] Used undeclared dependencies found:
[WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
[WARNING]    javax.inject:javax.inject:jar:1:compile
[WARNING] Unused declared dependencies found:
[WARNING]    org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
[WARNING]    org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
[WARNING]    org.hibernate:hibernate-validator:jar:4.2.0.Final:test
[WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
[WARNING]    org.jmock:jmock:jar:2.5.1:test

The question is (for this module but more generally for all others):
* Should we add slf4j and javax.inject reps in the pom.xml for this module? (for ex today slf4j and javax.inject are found in the component-api dep)

I think we should, wdyt?

Note that the "Unused declared dependencies found:" doesn't always generate correct results as is the case here. This is mostly because it's a static byte code check so any dep used at runtime will be considered unused.
See http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html

Thanks
-Vincent




_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Sergiu Dumitriu-2
On 08/12/2011 07:50 AM, Vincent Massol wrote:

> Hi devs,
>
> Running mvn dependency:dependency-analyze produces interesting results.
>
> For example:
>
> [INFO] ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
> [INFO] ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> …
> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @ xwiki-commons-properties ---
> [WARNING] Used undeclared dependencies found:
> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
> [WARNING]    javax.inject:javax.inject:jar:1:compile
> [WARNING] Unused declared dependencies found:
> [WARNING]    org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
> [WARNING]    org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
> [WARNING]    org.hibernate:hibernate-validator:jar:4.2.0.Final:test
> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
> [WARNING]    org.jmock:jmock:jar:2.5.1:test
>
> The question is (for this module but more generally for all others):
> * Should we add slf4j and javax.inject reps in the pom.xml for this module? (for ex today slf4j and javax.inject are found in the component-api dep)
>
> I think we should, wdyt?

+1 as well.

> Note that the "Unused declared dependencies found:" doesn't always generate correct results as is the case here. This is mostly because it's a static byte code check so any dep used at runtime will be considered unused.
> See http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html

Some of these dependencies are not used directly by us, but are needed
transitively by another library. For example, slf4j needs logback, which
we never use directly (although we don't really declare it in every
module that does logging). Hibernate needs us to pick a cache, a
connection pool, validator, and a bytecode manipulation utility. So yes,
we can safely ignore most of these false negatives, but we should still
try to remove those that are really wrongfully declared as dependencies.

> Thanks
> -Vincent
>
>
>
>
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs

--
Sergiu Dumitriu
http://purl.org/net/sergiu/
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

vmassol
Administrator

On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:

> On 08/12/2011 07:50 AM, Vincent Massol wrote:
>> Hi devs,
>>
>> Running mvn dependency:dependency-analyze produces interesting results.
>>
>> For example:
>>
>> [INFO] ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
>> [INFO] ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>> …
>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @ xwiki-commons-properties ---
>> [WARNING] Used undeclared dependencies found:
>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
>> [WARNING]    javax.inject:javax.inject:jar:1:compile
>> [WARNING] Unused declared dependencies found:
>> [WARNING]    org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
>> [WARNING]    org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
>> [WARNING]    org.hibernate:hibernate-validator:jar:4.2.0.Final:test
>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
>>
>> The question is (for this module but more generally for all others):
>> * Should we add slf4j and javax.inject reps in the pom.xml for this module? (for ex today slf4j and javax.inject are found in the component-api dep)
>>
>> I think we should, wdyt?
>
> +1 as well.

hmm actually we need to decide about the following:

* In order to simplify writing pom.xml for modules having components (i.e. depending on xwiki-commons-component-api) I had added the following to  xwiki-commons-component-api/pom.xml:

    <!-- Make it easy for components that wish to log - They don't have to explicitly import SLF4J -->
    <dependency>
      <groupId>org.slf4j</groupId>
      <artifactId>slf4j-api</artifactId>
    </dependency>

* In the same manner we have a dependency on javax.inject in xwiki-commons-component-api/pom.xml:

    <!-- We add this dependency here so that users of the Component API just need to depend on this artifact and
         don't have to explicitly add a dependency on javax.inject:java.inject. -->
    <dependency>
      <groupId>javax.inject</groupId>
      <artifactId>javax.inject</artifactId>
      <version>1</version>
    </dependency>

So the question is: do we want to force each module depending on xwiki-commons-component-api to have to declare an explicit dep on javax.inject and org.slf4j?

I'm not so sure about that…

WDYT?

Thanks
-Vincent

>> Note that the "Unused declared dependencies found:" doesn't always generate correct results as is the case here. This is mostly because it's a static byte code check so any dep used at runtime will be considered unused.
>> See http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
>
> Some of these dependencies are not used directly by us, but are needed
> transitively by another library. For example, slf4j needs logback, which
> we never use directly (although we don't really declare it in every
> module that does logging). Hibernate needs us to pick a cache, a
> connection pool, validator, and a bytecode manipulation utility. So yes,
> we can safely ignore most of these false negatives, but we should still
> try to remove those that are really wrongfully declared as dependencies.
>
>> Thanks
>> -Vincent

_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Sergiu Dumitriu-2
On 08/15/2011 03:25 PM, Vincent Massol wrote:

>
> On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
>
>> On 08/12/2011 07:50 AM, Vincent Massol wrote:
>>> Hi devs,
>>>
>>> Running mvn dependency:dependency-analyze produces interesting results.
>>>
>>> For example:
>>>
>>> [INFO] ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
>>> [INFO] ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>> …
>>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @ xwiki-commons-properties ---
>>> [WARNING] Used undeclared dependencies found:
>>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
>>> [WARNING]    javax.inject:javax.inject:jar:1:compile
>>> [WARNING] Unused declared dependencies found:
>>> [WARNING]    org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
>>> [WARNING]    org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
>>> [WARNING]    org.hibernate:hibernate-validator:jar:4.2.0.Final:test
>>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
>>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
>>>
>>> The question is (for this module but more generally for all others):
>>> * Should we add slf4j and javax.inject reps in the pom.xml for this module? (for ex today slf4j and javax.inject are found in the component-api dep)
>>>
>>> I think we should, wdyt?
>>
>> +1 as well.
>
> hmm actually we need to decide about the following:
>
> * In order to simplify writing pom.xml for modules having components (i.e. depending on xwiki-commons-component-api) I had added the following to  xwiki-commons-component-api/pom.xml:
>
>      <!-- Make it easy for components that wish to log - They don't have to explicitly import SLF4J -->
>      <dependency>
>        <groupId>org.slf4j</groupId>
>        <artifactId>slf4j-api</artifactId>
>      </dependency>
>
> * In the same manner we have a dependency on javax.inject in xwiki-commons-component-api/pom.xml:
>
>      <!-- We add this dependency here so that users of the Component API just need to depend on this artifact and
>           don't have to explicitly add a dependency on javax.inject:java.inject. -->
>      <dependency>
>        <groupId>javax.inject</groupId>
>        <artifactId>javax.inject</artifactId>
>        <version>1</version>
>      </dependency>
>
> So the question is: do we want to force each module depending on xwiki-commons-component-api to have to declare an explicit dep on javax.inject and org.slf4j?
>
> I'm not so sure about that…
>
> WDYT?

Hm, personally I'm in favor of Yes, but not very strongly.

These are utility dependencies, which don't add much value to the code;
they're part of the "infrastructure". So it would make sense to list
only "real" dependencies that influence the higher level code.

On the other hand, it's simpler to have just one clear rule: list all
used dependencies. This also makes it easier to hunt down and spot all
users of a library in case we need to change it, like we recently did
with log4j->slf4j.

Another thing just came to my mind in favor of not listing them: Maven
encourages reuse and convention-over-configuration, so it would be good
to define common infrastructure in just one place instead of
copy-pasting it everywhere.

> Thanks
> -Vincent
>
>>> Note that the "Unused declared dependencies found:" doesn't always generate correct results as is the case here. This is mostly because it's a static byte code check so any dep used at runtime will be considered unused.
>>> See http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
>>
>> Some of these dependencies are not used directly by us, but are needed
>> transitively by another library. For example, slf4j needs logback, which
>> we never use directly (although we don't really declare it in every
>> module that does logging). Hibernate needs us to pick a cache, a
>> connection pool, validator, and a bytecode manipulation utility. So yes,
>> we can safely ignore most of these false negatives, but we should still
>> try to remove those that are really wrongfully declared as dependencies.
>>
>>> Thanks
>>> -Vincent


--
Sergiu Dumitriu
http://purl.org/net/sergiu/
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Thomas Mortagne
Administrator
In reply to this post by vmassol
On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <[hidden email]> wrote:

>
> On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
>
>> On 08/12/2011 07:50 AM, Vincent Massol wrote:
>>> Hi devs,
>>>
>>> Running mvn dependency:dependency-analyze produces interesting results.
>>>
>>> For example:
>>>
>>> [INFO] ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
>>> [INFO] ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>> …
>>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @ xwiki-commons-properties ---
>>> [WARNING] Used undeclared dependencies found:
>>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
>>> [WARNING]    javax.inject:javax.inject:jar:1:compile
>>> [WARNING] Unused declared dependencies found:
>>> [WARNING]    org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
>>> [WARNING]    org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
>>> [WARNING]    org.hibernate:hibernate-validator:jar:4.2.0.Final:test
>>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
>>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
>>>
>>> The question is (for this module but more generally for all others):
>>> * Should we add slf4j and javax.inject reps in the pom.xml for this module? (for ex today slf4j and javax.inject are found in the component-api dep)
>>>
>>> I think we should, wdyt?
>>
>> +1 as well.
>
> hmm actually we need to decide about the following:
>
> * In order to simplify writing pom.xml for modules having components (i.e. depending on xwiki-commons-component-api) I had added the following to  xwiki-commons-component-api/pom.xml:
>
>    <!-- Make it easy for components that wish to log - They don't have to explicitly import SLF4J -->
>    <dependency>
>      <groupId>org.slf4j</groupId>
>      <artifactId>slf4j-api</artifactId>
>    </dependency>
>
> * In the same manner we have a dependency on javax.inject in xwiki-commons-component-api/pom.xml:
>
>    <!-- We add this dependency here so that users of the Component API just need to depend on this artifact and
>         don't have to explicitly add a dependency on javax.inject:java.inject. -->
>    <dependency>
>      <groupId>javax.inject</groupId>
>      <artifactId>javax.inject</artifactId>
>      <version>1</version>
>    </dependency>
>
> So the question is: do we want to force each module depending on xwiki-commons-component-api to have to declare an explicit dep on javax.inject and org.slf4j?
>
> I'm not so sure about that…

I'm -0 and near -1 to list this kind of dependencies, using slf4j or
javax.inject are what you HAVE TO use when you write an XWiki
component so it's redundant from my POV.

>
> WDYT?
>
> Thanks
> -Vincent
>
>>> Note that the "Unused declared dependencies found:" doesn't always generate correct results as is the case here. This is mostly because it's a static byte code check so any dep used at runtime will be considered unused.
>>> See http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
>>
>> Some of these dependencies are not used directly by us, but are needed
>> transitively by another library. For example, slf4j needs logback, which
>> we never use directly (although we don't really declare it in every
>> module that does logging). Hibernate needs us to pick a cache, a
>> connection pool, validator, and a bytecode manipulation utility. So yes,
>> we can safely ignore most of these false negatives, but we should still
>> try to remove those that are really wrongfully declared as dependencies.
>>
>>> Thanks
>>> -Vincent
>
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs
>



--
Thomas Mortagne
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Denis Gervalle-2
Hi devs,

I am reviving this proposal since we never came to a conclusion, and I have
the feeling that our deps become more and more convoluted.

To resume what I get from past history with my own vision:

1) Since our modules are getting really modular, it should never silently
depends on transitive dependency of another module (with IMO some
exception, see 3). So any undeclared deps found by dependency:analyse
should be explicitly declare in the effective pom (Vincent POV as well)
2) Apply maven principle, we should reuse and apply
convention-over-configuration
over configuration, so common dependency like slf4j, xwiki-commons-stability
? ... should be in a high level parent pom
3) We may rely on some very tight transitive dependency, for exemple, it
would be really curious that xwiki-commons-component-api stop providing
javax.inject, or that xwki-commons-test-components stop providing junit,
mockito, and al.

It would be nice to add those rules in our best practice and to always
check our pom upon finishing changes in a module. The best would be to
enforce those rules, but this is not easy since static analysis is limited
and could create false positive.

WDYT ?


On Mon, Aug 15, 2011 at 10:31 PM, Thomas Mortagne <[hidden email]
> wrote:

> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <[hidden email]>
> wrote:
> >
> > On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
> >
> >> On 08/12/2011 07:50 AM, Vincent Massol wrote:
> >>> Hi devs,
> >>>
> >>> Running mvn dependency:dependency-analyze produces interesting results.
> >>>
> >>> For example:
> >>>
> >>> [INFO]
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> >>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
> >>> [INFO]
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> >>> …
> >>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @
> xwiki-commons-properties ---
> >>> [WARNING] Used undeclared dependencies found:
> >>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
> >>> [WARNING]    javax.inject:javax.inject:jar:1:compile
> >>> [WARNING] Unused declared dependencies found:
> >>> [WARNING]
>  org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
> >>> [WARNING]    org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
> >>> [WARNING]    org.hibernate:hibernate-validator:jar:4.2.0.Final:test
> >>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
> >>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
> >>>
> >>> The question is (for this module but more generally for all others):
> >>> * Should we add slf4j and javax.inject reps in the pom.xml for this
> module? (for ex today slf4j and javax.inject are found in the component-api
> dep)
> >>>
> >>> I think we should, wdyt?
> >>
> >> +1 as well.
> >
> > hmm actually we need to decide about the following:
> >
> > * In order to simplify writing pom.xml for modules having components
> (i.e. depending on xwiki-commons-component-api) I had added the following
> to  xwiki-commons-component-api/pom.xml:
> >
> >    <!-- Make it easy for components that wish to log - They don't have
> to explicitly import SLF4J -->
> >    <dependency>
> >      <groupId>org.slf4j</groupId>
> >      <artifactId>slf4j-api</artifactId>
> >    </dependency>
> >
> > * In the same manner we have a dependency on javax.inject in
> xwiki-commons-component-api/pom.xml:
> >
> >    <!-- We add this dependency here so that users of the Component API
> just need to depend on this artifact and
> >         don't have to explicitly add a dependency on
> javax.inject:java.inject. -->
> >    <dependency>
> >      <groupId>javax.inject</groupId>
> >      <artifactId>javax.inject</artifactId>
> >      <version>1</version>
> >    </dependency>
> >
> > So the question is: do we want to force each module depending on
> xwiki-commons-component-api to have to declare an explicit dep on
> javax.inject and org.slf4j?
> >
> > I'm not so sure about that…
>
> I'm -0 and near -1 to list this kind of dependencies, using slf4j or
> javax.inject are what you HAVE TO use when you write an XWiki
> component so it's redundant from my POV.
>
> >
> > WDYT?
> >
> > Thanks
> > -Vincent
> >
> >>> Note that the "Unused declared dependencies found:" doesn't always
> generate correct results as is the case here. This is mostly because it's a
> static byte code check so any dep used at runtime will be considered unused.
> >>> See
> http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
> >>
> >> Some of these dependencies are not used directly by us, but are needed
> >> transitively by another library. For example, slf4j needs logback, which
> >> we never use directly (although we don't really declare it in every
> >> module that does logging). Hibernate needs us to pick a cache, a
> >> connection pool, validator, and a bytecode manipulation utility. So yes,
> >> we can safely ignore most of these false negatives, but we should still
> >> try to remove those that are really wrongfully declared as dependencies.
> >>
> >>> Thanks
> >>> -Vincent
> >
> > _______________________________________________
> > devs mailing list
> > [hidden email]
> > http://lists.xwiki.org/mailman/listinfo/devs
> >
>
>
>
> --
> Thomas Mortagne
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs
>



--
Denis Gervalle
SOFTEC sa - CEO
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Thomas Mortagne
Administrator
For me the rule to apply is simple: when you require dependency A
because of another dependency B (B expose A in it's API, you implement
an interface of A to be found by B, etc.) you should not explicitly
depend on A.

On Thu, Jun 5, 2014 at 11:32 AM, Denis Gervalle <[hidden email]> wrote:

> Hi devs,
>
> I am reviving this proposal since we never came to a conclusion, and I have
> the feeling that our deps become more and more convoluted.
>
> To resume what I get from past history with my own vision:
>
> 1) Since our modules are getting really modular, it should never silently
> depends on transitive dependency of another module (with IMO some
> exception, see 3). So any undeclared deps found by dependency:analyse
> should be explicitly declare in the effective pom (Vincent POV as well)
> 2) Apply maven principle, we should reuse and apply
> convention-over-configuration
> over configuration, so common dependency like slf4j, xwiki-commons-stability
> ? ... should be in a high level parent pom
> 3) We may rely on some very tight transitive dependency, for exemple, it
> would be really curious that xwiki-commons-component-api stop providing
> javax.inject, or that xwki-commons-test-components stop providing junit,
> mockito, and al.
>
> It would be nice to add those rules in our best practice and to always
> check our pom upon finishing changes in a module. The best would be to
> enforce those rules, but this is not easy since static analysis is limited
> and could create false positive.
>
> WDYT ?
>
>
> On Mon, Aug 15, 2011 at 10:31 PM, Thomas Mortagne <[hidden email]
>> wrote:
>
>> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <[hidden email]>
>> wrote:
>> >
>> > On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
>> >
>> >> On 08/12/2011 07:50 AM, Vincent Massol wrote:
>> >>> Hi devs,
>> >>>
>> >>> Running mvn dependency:dependency-analyze produces interesting results.
>> >>>
>> >>> For example:
>> >>>
>> >>> [INFO]
>> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>> >>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
>> >>> [INFO]
>> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>> >>> …
>> >>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @
>> xwiki-commons-properties ---
>> >>> [WARNING] Used undeclared dependencies found:
>> >>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
>> >>> [WARNING]    javax.inject:javax.inject:jar:1:compile
>> >>> [WARNING] Unused declared dependencies found:
>> >>> [WARNING]
>>  org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
>> >>> [WARNING]    org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
>> >>> [WARNING]    org.hibernate:hibernate-validator:jar:4.2.0.Final:test
>> >>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
>> >>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
>> >>>
>> >>> The question is (for this module but more generally for all others):
>> >>> * Should we add slf4j and javax.inject reps in the pom.xml for this
>> module? (for ex today slf4j and javax.inject are found in the component-api
>> dep)
>> >>>
>> >>> I think we should, wdyt?
>> >>
>> >> +1 as well.
>> >
>> > hmm actually we need to decide about the following:
>> >
>> > * In order to simplify writing pom.xml for modules having components
>> (i.e. depending on xwiki-commons-component-api) I had added the following
>> to  xwiki-commons-component-api/pom.xml:
>> >
>> >    <!-- Make it easy for components that wish to log - They don't have
>> to explicitly import SLF4J -->
>> >    <dependency>
>> >      <groupId>org.slf4j</groupId>
>> >      <artifactId>slf4j-api</artifactId>
>> >    </dependency>
>> >
>> > * In the same manner we have a dependency on javax.inject in
>> xwiki-commons-component-api/pom.xml:
>> >
>> >    <!-- We add this dependency here so that users of the Component API
>> just need to depend on this artifact and
>> >         don't have to explicitly add a dependency on
>> javax.inject:java.inject. -->
>> >    <dependency>
>> >      <groupId>javax.inject</groupId>
>> >      <artifactId>javax.inject</artifactId>
>> >      <version>1</version>
>> >    </dependency>
>> >
>> > So the question is: do we want to force each module depending on
>> xwiki-commons-component-api to have to declare an explicit dep on
>> javax.inject and org.slf4j?
>> >
>> > I'm not so sure about that…
>>
>> I'm -0 and near -1 to list this kind of dependencies, using slf4j or
>> javax.inject are what you HAVE TO use when you write an XWiki
>> component so it's redundant from my POV.
>>
>> >
>> > WDYT?
>> >
>> > Thanks
>> > -Vincent
>> >
>> >>> Note that the "Unused declared dependencies found:" doesn't always
>> generate correct results as is the case here. This is mostly because it's a
>> static byte code check so any dep used at runtime will be considered unused.
>> >>> See
>> http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
>> >>
>> >> Some of these dependencies are not used directly by us, but are needed
>> >> transitively by another library. For example, slf4j needs logback, which
>> >> we never use directly (although we don't really declare it in every
>> >> module that does logging). Hibernate needs us to pick a cache, a
>> >> connection pool, validator, and a bytecode manipulation utility. So yes,
>> >> we can safely ignore most of these false negatives, but we should still
>> >> try to remove those that are really wrongfully declared as dependencies.
>> >>
>> >>> Thanks
>> >>> -Vincent
>> >
>> > _______________________________________________
>> > devs mailing list
>> > [hidden email]
>> > http://lists.xwiki.org/mailman/listinfo/devs
>> >
>>
>>
>>
>> --
>> Thomas Mortagne
>> _______________________________________________
>> devs mailing list
>> [hidden email]
>> http://lists.xwiki.org/mailman/listinfo/devs
>>
>
>
>
> --
> Denis Gervalle
> SOFTEC sa - CEO
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs



--
Thomas Mortagne
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Denis Gervalle-2
What happen if you also use dependency A not just because of B ?
How do you determine "because of B" ?

And what to you think of xwiki-commons-test-component that is a deps of
xwiki-platform-core ?
Should we remove it ? What about deps for logging ?
And could we add xwiki-commons-stability (probably provided scope) to a
high level pom to avoid adding/removing it all the time ? (or forget it,
since it come with xwiki-commons-component-api currently) ?

WDYT ?



On Thu, Jun 5, 2014 at 11:42 AM, Thomas Mortagne <[hidden email]>
wrote:

> For me the rule to apply is simple: when you require dependency A
> because of another dependency B (B expose A in it's API, you implement
> an interface of A to be found by B, etc.) you should not explicitly
> depend on A.
>
> On Thu, Jun 5, 2014 at 11:32 AM, Denis Gervalle <[hidden email]> wrote:
> > Hi devs,
> >
> > I am reviving this proposal since we never came to a conclusion, and I
> have
> > the feeling that our deps become more and more convoluted.
> >
> > To resume what I get from past history with my own vision:
> >
> > 1) Since our modules are getting really modular, it should never silently
> > depends on transitive dependency of another module (with IMO some
> > exception, see 3). So any undeclared deps found by dependency:analyse
> > should be explicitly declare in the effective pom (Vincent POV as well)
> > 2) Apply maven principle, we should reuse and apply
> > convention-over-configuration
> > over configuration, so common dependency like slf4j,
> xwiki-commons-stability
> > ? ... should be in a high level parent pom
> > 3) We may rely on some very tight transitive dependency, for exemple, it
> > would be really curious that xwiki-commons-component-api stop providing
> > javax.inject, or that xwki-commons-test-components stop providing junit,
> > mockito, and al.
> >
> > It would be nice to add those rules in our best practice and to always
> > check our pom upon finishing changes in a module. The best would be to
> > enforce those rules, but this is not easy since static analysis is
> limited
> > and could create false positive.
> >
> > WDYT ?
> >
> >
> > On Mon, Aug 15, 2011 at 10:31 PM, Thomas Mortagne <
> [hidden email]
> >> wrote:
> >
> >> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <[hidden email]>
> >> wrote:
> >> >
> >> > On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
> >> >
> >> >> On 08/12/2011 07:50 AM, Vincent Massol wrote:
> >> >>> Hi devs,
> >> >>>
> >> >>> Running mvn dependency:dependency-analyze produces interesting
> results.
> >> >>>
> >> >>> For example:
> >> >>>
> >> >>> [INFO]
> >>
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> >> >>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
> >> >>> [INFO]
> >>
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> >> >>> …
> >> >>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @
> >> xwiki-commons-properties ---
> >> >>> [WARNING] Used undeclared dependencies found:
> >> >>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
> >> >>> [WARNING]    javax.inject:javax.inject:jar:1:compile
> >> >>> [WARNING] Unused declared dependencies found:
> >> >>> [WARNING]
> >>  org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
> >> >>> [WARNING]
>  org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
> >> >>> [WARNING]    org.hibernate:hibernate-validator:jar:4.2.0.Final:test
> >> >>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
> >> >>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
> >> >>>
> >> >>> The question is (for this module but more generally for all others):
> >> >>> * Should we add slf4j and javax.inject reps in the pom.xml for this
> >> module? (for ex today slf4j and javax.inject are found in the
> component-api
> >> dep)
> >> >>>
> >> >>> I think we should, wdyt?
> >> >>
> >> >> +1 as well.
> >> >
> >> > hmm actually we need to decide about the following:
> >> >
> >> > * In order to simplify writing pom.xml for modules having components
> >> (i.e. depending on xwiki-commons-component-api) I had added the
> following
> >> to  xwiki-commons-component-api/pom.xml:
> >> >
> >> >    <!-- Make it easy for components that wish to log - They don't have
> >> to explicitly import SLF4J -->
> >> >    <dependency>
> >> >      <groupId>org.slf4j</groupId>
> >> >      <artifactId>slf4j-api</artifactId>
> >> >    </dependency>
> >> >
> >> > * In the same manner we have a dependency on javax.inject in
> >> xwiki-commons-component-api/pom.xml:
> >> >
> >> >    <!-- We add this dependency here so that users of the Component API
> >> just need to depend on this artifact and
> >> >         don't have to explicitly add a dependency on
> >> javax.inject:java.inject. -->
> >> >    <dependency>
> >> >      <groupId>javax.inject</groupId>
> >> >      <artifactId>javax.inject</artifactId>
> >> >      <version>1</version>
> >> >    </dependency>
> >> >
> >> > So the question is: do we want to force each module depending on
> >> xwiki-commons-component-api to have to declare an explicit dep on
> >> javax.inject and org.slf4j?
> >> >
> >> > I'm not so sure about that…
> >>
> >> I'm -0 and near -1 to list this kind of dependencies, using slf4j or
> >> javax.inject are what you HAVE TO use when you write an XWiki
> >> component so it's redundant from my POV.
> >>
> >> >
> >> > WDYT?
> >> >
> >> > Thanks
> >> > -Vincent
> >> >
> >> >>> Note that the "Unused declared dependencies found:" doesn't always
> >> generate correct results as is the case here. This is mostly because
> it's a
> >> static byte code check so any dep used at runtime will be considered
> unused.
> >> >>> See
> >>
> http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
> >> >>
> >> >> Some of these dependencies are not used directly by us, but are
> needed
> >> >> transitively by another library. For example, slf4j needs logback,
> which
> >> >> we never use directly (although we don't really declare it in every
> >> >> module that does logging). Hibernate needs us to pick a cache, a
> >> >> connection pool, validator, and a bytecode manipulation utility. So
> yes,
> >> >> we can safely ignore most of these false negatives, but we should
> still
> >> >> try to remove those that are really wrongfully declared as
> dependencies.
> >> >>
> >> >>> Thanks
> >> >>> -Vincent
> >> >
> >> > _______________________________________________
> >> > devs mailing list
> >> > [hidden email]
> >> > http://lists.xwiki.org/mailman/listinfo/devs
> >> >
> >>
> >>
> >>
> >> --
> >> Thomas Mortagne
> >> _______________________________________________
> >> devs mailing list
> >> [hidden email]
> >> http://lists.xwiki.org/mailman/listinfo/devs
> >>
> >
> >
> >
> > --
> > Denis Gervalle
> > SOFTEC sa - CEO
> > _______________________________________________
> > devs mailing list
> > [hidden email]
> > http://lists.xwiki.org/mailman/listinfo/devs
>
>
>
> --
> Thomas Mortagne
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs
>



--
Denis Gervalle
SOFTEC sa - CEO
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Thomas Mortagne
Administrator
On Thu, Jun 5, 2014 at 2:14 PM, Denis Gervalle <[hidden email]> wrote:
> What happen if you also use dependency A not just because of B ?

You put a dependency on A.

> How do you determine "because of B" ?

By thinking.

>
> And what to you think of xwiki-commons-test-component that is a deps of
> xwiki-platform-core ?

It's wrong IMO. Any forced dependency is wrong IMO.

> Should we remove it ?

Yes we should remove it.

> What about deps for logging ?

Depends how you use it, the logger used with @Inject is an official
feature of our component framework so xwiki-commons-component-api
should be enough.

> And could we add xwiki-commons-stability (probably provided scope) to a
> high level pom to avoid adding/removing it all the time ? (or forget it,
> since it come with xwiki-commons-component-api currently) ?

It's far from being used everywhere and there is no rule forcing to
use it, you set @Unstable when an API is unstable, it's not forbidden
to not go through @Unstable. Plus you are supposed to remove that
annotation after some time.

>
> WDYT ?
>
>
>
> On Thu, Jun 5, 2014 at 11:42 AM, Thomas Mortagne <[hidden email]>
> wrote:
>
>> For me the rule to apply is simple: when you require dependency A
>> because of another dependency B (B expose A in it's API, you implement
>> an interface of A to be found by B, etc.) you should not explicitly
>> depend on A.
>>
>> On Thu, Jun 5, 2014 at 11:32 AM, Denis Gervalle <[hidden email]> wrote:
>> > Hi devs,
>> >
>> > I am reviving this proposal since we never came to a conclusion, and I
>> have
>> > the feeling that our deps become more and more convoluted.
>> >
>> > To resume what I get from past history with my own vision:
>> >
>> > 1) Since our modules are getting really modular, it should never silently
>> > depends on transitive dependency of another module (with IMO some
>> > exception, see 3). So any undeclared deps found by dependency:analyse
>> > should be explicitly declare in the effective pom (Vincent POV as well)
>> > 2) Apply maven principle, we should reuse and apply
>> > convention-over-configuration
>> > over configuration, so common dependency like slf4j,
>> xwiki-commons-stability
>> > ? ... should be in a high level parent pom
>> > 3) We may rely on some very tight transitive dependency, for exemple, it
>> > would be really curious that xwiki-commons-component-api stop providing
>> > javax.inject, or that xwki-commons-test-components stop providing junit,
>> > mockito, and al.
>> >
>> > It would be nice to add those rules in our best practice and to always
>> > check our pom upon finishing changes in a module. The best would be to
>> > enforce those rules, but this is not easy since static analysis is
>> limited
>> > and could create false positive.
>> >
>> > WDYT ?
>> >
>> >
>> > On Mon, Aug 15, 2011 at 10:31 PM, Thomas Mortagne <
>> [hidden email]
>> >> wrote:
>> >
>> >> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <[hidden email]>
>> >> wrote:
>> >> >
>> >> > On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
>> >> >
>> >> >> On 08/12/2011 07:50 AM, Vincent Massol wrote:
>> >> >>> Hi devs,
>> >> >>>
>> >> >>> Running mvn dependency:dependency-analyze produces interesting
>> results.
>> >> >>>
>> >> >>> For example:
>> >> >>>
>> >> >>> [INFO]
>> >>
>> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>> >> >>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
>> >> >>> [INFO]
>> >>
>> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>> >> >>> …
>> >> >>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @
>> >> xwiki-commons-properties ---
>> >> >>> [WARNING] Used undeclared dependencies found:
>> >> >>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
>> >> >>> [WARNING]    javax.inject:javax.inject:jar:1:compile
>> >> >>> [WARNING] Unused declared dependencies found:
>> >> >>> [WARNING]
>> >>  org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
>> >> >>> [WARNING]
>>  org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
>> >> >>> [WARNING]    org.hibernate:hibernate-validator:jar:4.2.0.Final:test
>> >> >>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
>> >> >>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
>> >> >>>
>> >> >>> The question is (for this module but more generally for all others):
>> >> >>> * Should we add slf4j and javax.inject reps in the pom.xml for this
>> >> module? (for ex today slf4j and javax.inject are found in the
>> component-api
>> >> dep)
>> >> >>>
>> >> >>> I think we should, wdyt?
>> >> >>
>> >> >> +1 as well.
>> >> >
>> >> > hmm actually we need to decide about the following:
>> >> >
>> >> > * In order to simplify writing pom.xml for modules having components
>> >> (i.e. depending on xwiki-commons-component-api) I had added the
>> following
>> >> to  xwiki-commons-component-api/pom.xml:
>> >> >
>> >> >    <!-- Make it easy for components that wish to log - They don't have
>> >> to explicitly import SLF4J -->
>> >> >    <dependency>
>> >> >      <groupId>org.slf4j</groupId>
>> >> >      <artifactId>slf4j-api</artifactId>
>> >> >    </dependency>
>> >> >
>> >> > * In the same manner we have a dependency on javax.inject in
>> >> xwiki-commons-component-api/pom.xml:
>> >> >
>> >> >    <!-- We add this dependency here so that users of the Component API
>> >> just need to depend on this artifact and
>> >> >         don't have to explicitly add a dependency on
>> >> javax.inject:java.inject. -->
>> >> >    <dependency>
>> >> >      <groupId>javax.inject</groupId>
>> >> >      <artifactId>javax.inject</artifactId>
>> >> >      <version>1</version>
>> >> >    </dependency>
>> >> >
>> >> > So the question is: do we want to force each module depending on
>> >> xwiki-commons-component-api to have to declare an explicit dep on
>> >> javax.inject and org.slf4j?
>> >> >
>> >> > I'm not so sure about that…
>> >>
>> >> I'm -0 and near -1 to list this kind of dependencies, using slf4j or
>> >> javax.inject are what you HAVE TO use when you write an XWiki
>> >> component so it's redundant from my POV.
>> >>
>> >> >
>> >> > WDYT?
>> >> >
>> >> > Thanks
>> >> > -Vincent
>> >> >
>> >> >>> Note that the "Unused declared dependencies found:" doesn't always
>> >> generate correct results as is the case here. This is mostly because
>> it's a
>> >> static byte code check so any dep used at runtime will be considered
>> unused.
>> >> >>> See
>> >>
>> http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
>> >> >>
>> >> >> Some of these dependencies are not used directly by us, but are
>> needed
>> >> >> transitively by another library. For example, slf4j needs logback,
>> which
>> >> >> we never use directly (although we don't really declare it in every
>> >> >> module that does logging). Hibernate needs us to pick a cache, a
>> >> >> connection pool, validator, and a bytecode manipulation utility. So
>> yes,
>> >> >> we can safely ignore most of these false negatives, but we should
>> still
>> >> >> try to remove those that are really wrongfully declared as
>> dependencies.
>> >> >>
>> >> >>> Thanks
>> >> >>> -Vincent
>> >> >
>> >> > _______________________________________________
>> >> > devs mailing list
>> >> > [hidden email]
>> >> > http://lists.xwiki.org/mailman/listinfo/devs
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Thomas Mortagne
>> >> _______________________________________________
>> >> devs mailing list
>> >> [hidden email]
>> >> http://lists.xwiki.org/mailman/listinfo/devs
>> >>
>> >
>> >
>> >
>> > --
>> > Denis Gervalle
>> > SOFTEC sa - CEO
>> > _______________________________________________
>> > devs mailing list
>> > [hidden email]
>> > http://lists.xwiki.org/mailman/listinfo/devs
>>
>>
>>
>> --
>> Thomas Mortagne
>> _______________________________________________
>> devs mailing list
>> [hidden email]
>> http://lists.xwiki.org/mailman/listinfo/devs
>>
>
>
>
> --
> Denis Gervalle
> SOFTEC sa - CEO
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs



--
Thomas Mortagne
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Denis Gervalle-2
On Thu, Jun 5, 2014 at 2:27 PM, Thomas Mortagne <[hidden email]>
wrote:

> On Thu, Jun 5, 2014 at 2:14 PM, Denis Gervalle <[hidden email]> wrote:
> > What happen if you also use dependency A not just because of B ?
>
> You put a dependency on A.
>

But you may not see that so easily when you change a few line in an
existing module. Nothing will complains until you remove your deps to B.


>
> > How do you determine "because of B" ?
>
> By thinking.
>

Ok, I rephrase my question.
Could you define what you consider a usage of dependency A because of B and
the opposite ?


>
> >
> > And what to you think of xwiki-commons-test-component that is a deps of
> > xwiki-platform-core ?
>
> It's wrong IMO. Any forced dependency is wrong IMO.
>
> > Should we remove it ?
>
> Yes we should remove it.
>

Why do we get it ? Its removal could become an nightmare... but if we agree
on that, we should remove it ASAP.


>
> > What about deps for logging ?
>
> Depends how you use it, the logger used with @Inject is an official
> feature of our component framework so xwiki-commons-component-api
> should be enough.
>
> > And could we add xwiki-commons-stability (probably provided scope) to a
> > high level pom to avoid adding/removing it all the time ? (or forget it,
> > since it come with xwiki-commons-component-api currently) ?
>
> It's far from being used everywhere and there is no rule forcing to
> use it, you set @Unstable when an API is unstable, it's not forbidden
> to not go through @Unstable. Plus you are supposed to remove that
> annotation after some time.
>

Ok, so not deps at any scope in any high level poms. This seems opposite to
what Sergiu proposed, but it would be nice to agree on a rule.

To sum up, currently I am not sure the exception rule "because of" is clear
enough to not create confusion. I also agree with Sergiu that we should
list all (no warning of used deps not declared in dependency:analysis),
this make the rule clear at least. I am not against factoring common
infrastructure in a single place, but Thomas seems to be clearly -1.

It would be nice to have more feedback from other committers ! This is not
a minor aspect of our best practice IMO.


>
> >
> > WDYT ?
> >
> >
> >
> > On Thu, Jun 5, 2014 at 11:42 AM, Thomas Mortagne <
> [hidden email]>
> > wrote:
> >
> >> For me the rule to apply is simple: when you require dependency A
> >> because of another dependency B (B expose A in it's API, you implement
> >> an interface of A to be found by B, etc.) you should not explicitly
> >> depend on A.
> >>
> >> On Thu, Jun 5, 2014 at 11:32 AM, Denis Gervalle <[hidden email]> wrote:
> >> > Hi devs,
> >> >
> >> > I am reviving this proposal since we never came to a conclusion, and I
> >> have
> >> > the feeling that our deps become more and more convoluted.
> >> >
> >> > To resume what I get from past history with my own vision:
> >> >
> >> > 1) Since our modules are getting really modular, it should never
> silently
> >> > depends on transitive dependency of another module (with IMO some
> >> > exception, see 3). So any undeclared deps found by dependency:analyse
> >> > should be explicitly declare in the effective pom (Vincent POV as
> well)
> >> > 2) Apply maven principle, we should reuse and apply
> >> > convention-over-configuration
> >> > over configuration, so common dependency like slf4j,
> >> xwiki-commons-stability
> >> > ? ... should be in a high level parent pom
> >> > 3) We may rely on some very tight transitive dependency, for exemple,
> it
> >> > would be really curious that xwiki-commons-component-api stop
> providing
> >> > javax.inject, or that xwki-commons-test-components stop providing
> junit,
> >> > mockito, and al.
> >> >
> >> > It would be nice to add those rules in our best practice and to always
> >> > check our pom upon finishing changes in a module. The best would be to
> >> > enforce those rules, but this is not easy since static analysis is
> >> limited
> >> > and could create false positive.
> >> >
> >> > WDYT ?
> >> >
> >> >
> >> > On Mon, Aug 15, 2011 at 10:31 PM, Thomas Mortagne <
> >> [hidden email]
> >> >> wrote:
> >> >
> >> >> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <[hidden email]>
> >> >> wrote:
> >> >> >
> >> >> > On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
> >> >> >
> >> >> >> On 08/12/2011 07:50 AM, Vincent Massol wrote:
> >> >> >>> Hi devs,
> >> >> >>>
> >> >> >>> Running mvn dependency:dependency-analyze produces interesting
> >> results.
> >> >> >>>
> >> >> >>> For example:
> >> >> >>>
> >> >> >>> [INFO]
> >> >>
> >>
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> >> >> >>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
> >> >> >>> [INFO]
> >> >>
> >>
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> >> >> >>> …
> >> >> >>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @
> >> >> xwiki-commons-properties ---
> >> >> >>> [WARNING] Used undeclared dependencies found:
> >> >> >>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
> >> >> >>> [WARNING]    javax.inject:javax.inject:jar:1:compile
> >> >> >>> [WARNING] Unused declared dependencies found:
> >> >> >>> [WARNING]
> >> >>
>  org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
> >> >> >>> [WARNING]
> >>  org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
> >> >> >>> [WARNING]
>  org.hibernate:hibernate-validator:jar:4.2.0.Final:test
> >> >> >>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
> >> >> >>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
> >> >> >>>
> >> >> >>> The question is (for this module but more generally for all
> others):
> >> >> >>> * Should we add slf4j and javax.inject reps in the pom.xml for
> this
> >> >> module? (for ex today slf4j and javax.inject are found in the
> >> component-api
> >> >> dep)
> >> >> >>>
> >> >> >>> I think we should, wdyt?
> >> >> >>
> >> >> >> +1 as well.
> >> >> >
> >> >> > hmm actually we need to decide about the following:
> >> >> >
> >> >> > * In order to simplify writing pom.xml for modules having
> components
> >> >> (i.e. depending on xwiki-commons-component-api) I had added the
> >> following
> >> >> to  xwiki-commons-component-api/pom.xml:
> >> >> >
> >> >> >    <!-- Make it easy for components that wish to log - They don't
> have
> >> >> to explicitly import SLF4J -->
> >> >> >    <dependency>
> >> >> >      <groupId>org.slf4j</groupId>
> >> >> >      <artifactId>slf4j-api</artifactId>
> >> >> >    </dependency>
> >> >> >
> >> >> > * In the same manner we have a dependency on javax.inject in
> >> >> xwiki-commons-component-api/pom.xml:
> >> >> >
> >> >> >    <!-- We add this dependency here so that users of the Component
> API
> >> >> just need to depend on this artifact and
> >> >> >         don't have to explicitly add a dependency on
> >> >> javax.inject:java.inject. -->
> >> >> >    <dependency>
> >> >> >      <groupId>javax.inject</groupId>
> >> >> >      <artifactId>javax.inject</artifactId>
> >> >> >      <version>1</version>
> >> >> >    </dependency>
> >> >> >
> >> >> > So the question is: do we want to force each module depending on
> >> >> xwiki-commons-component-api to have to declare an explicit dep on
> >> >> javax.inject and org.slf4j?
> >> >> >
> >> >> > I'm not so sure about that…
> >> >>
> >> >> I'm -0 and near -1 to list this kind of dependencies, using slf4j or
> >> >> javax.inject are what you HAVE TO use when you write an XWiki
> >> >> component so it's redundant from my POV.
> >> >>
> >> >> >
> >> >> > WDYT?
> >> >> >
> >> >> > Thanks
> >> >> > -Vincent
> >> >> >
> >> >> >>> Note that the "Unused declared dependencies found:" doesn't
> always
> >> >> generate correct results as is the case here. This is mostly because
> >> it's a
> >> >> static byte code check so any dep used at runtime will be considered
> >> unused.
> >> >> >>> See
> >> >>
> >>
> http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
> >> >> >>
> >> >> >> Some of these dependencies are not used directly by us, but are
> >> needed
> >> >> >> transitively by another library. For example, slf4j needs logback,
> >> which
> >> >> >> we never use directly (although we don't really declare it in
> every
> >> >> >> module that does logging). Hibernate needs us to pick a cache, a
> >> >> >> connection pool, validator, and a bytecode manipulation utility.
> So
> >> yes,
> >> >> >> we can safely ignore most of these false negatives, but we should
> >> still
> >> >> >> try to remove those that are really wrongfully declared as
> >> dependencies.
> >> >> >>
> >> >> >>> Thanks
> >> >> >>> -Vincent
> >> >> >
> >> >> > _______________________________________________
> >> >> > devs mailing list
> >> >> > [hidden email]
> >> >> > http://lists.xwiki.org/mailman/listinfo/devs
> >> >> >
> >> >>
> >> >>
> >> >>
> >> >> --
> >> >> Thomas Mortagne
> >> >> _______________________________________________
> >> >> devs mailing list
> >> >> [hidden email]
> >> >> http://lists.xwiki.org/mailman/listinfo/devs
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Denis Gervalle
> >> > SOFTEC sa - CEO
> >> > _______________________________________________
> >> > devs mailing list
> >> > [hidden email]
> >> > http://lists.xwiki.org/mailman/listinfo/devs
> >>
> >>
> >>
> >> --
> >> Thomas Mortagne
> >> _______________________________________________
> >> devs mailing list
> >> [hidden email]
> >> http://lists.xwiki.org/mailman/listinfo/devs
> >>
> >
> >
> >
> > --
> > Denis Gervalle
> > SOFTEC sa - CEO
> > _______________________________________________
> > devs mailing list
> > [hidden email]
> > http://lists.xwiki.org/mailman/listinfo/devs
>
>
>
> --
> Thomas Mortagne
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs
>



--
Denis Gervalle
SOFTEC sa - CEO
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Roman Muntyanu
>> Ok, I rephrase my question.
>> Could you define what you consider a usage of dependency A because of B and the opposite?

I'll dare explaining

Indirect dependency (no dependency on A should be defined in C):
C ----> B ----> A
  * C uses classes from B
  * B uses classes from A
  * C does NOT use classes from A

Direct dependency (explicit dependency on A should be defined in C)
C ----> B ----> A
 \_ _ _ _ _ _ _^
  * C uses classes from B
  * B uses classes from A
  * C uses classes from A

-----Original Message-----
From: devs [mailto:[hidden email]] On Behalf Of Denis Gervalle
Sent: Thursday, June 05, 2014 16:10 PM
To: XWiki Developers
Subject: Re: [xwiki-devs] [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

On Thu, Jun 5, 2014 at 2:27 PM, Thomas Mortagne <[hidden email]>
wrote:

> On Thu, Jun 5, 2014 at 2:14 PM, Denis Gervalle <[hidden email]> wrote:
> > What happen if you also use dependency A not just because of B ?
>
> You put a dependency on A.
>

But you may not see that so easily when you change a few line in an existing module. Nothing will complains until you remove your deps to B.


>
> > How do you determine "because of B" ?
>
> By thinking.
>

Ok, I rephrase my question.
Could you define what you consider a usage of dependency A because of B and the opposite ?


>
> >
> > And what to you think of xwiki-commons-test-component that is a deps
> > of xwiki-platform-core ?
>
> It's wrong IMO. Any forced dependency is wrong IMO.
>
> > Should we remove it ?
>
> Yes we should remove it.
>

Why do we get it ? Its removal could become an nightmare... but if we agree on that, we should remove it ASAP.


>
> > What about deps for logging ?
>
> Depends how you use it, the logger used with @Inject is an official
> feature of our component framework so xwiki-commons-component-api
> should be enough.
>
> > And could we add xwiki-commons-stability (probably provided scope)
> > to a high level pom to avoid adding/removing it all the time ? (or
> > forget it, since it come with xwiki-commons-component-api currently) ?
>
> It's far from being used everywhere and there is no rule forcing to
> use it, you set @Unstable when an API is unstable, it's not forbidden
> to not go through @Unstable. Plus you are supposed to remove that
> annotation after some time.
>

Ok, so not deps at any scope in any high level poms. This seems opposite to what Sergiu proposed, but it would be nice to agree on a rule.

To sum up, currently I am not sure the exception rule "because of" is clear enough to not create confusion. I also agree with Sergiu that we should list all (no warning of used deps not declared in dependency:analysis), this make the rule clear at least. I am not against factoring common infrastructure in a single place, but Thomas seems to be clearly -1.

It would be nice to have more feedback from other committers ! This is not a minor aspect of our best practice IMO.


>
> >
> > WDYT ?
> >
> >
> >
> > On Thu, Jun 5, 2014 at 11:42 AM, Thomas Mortagne <
> [hidden email]>
> > wrote:
> >
> >> For me the rule to apply is simple: when you require dependency A
> >> because of another dependency B (B expose A in it's API, you
> >> implement an interface of A to be found by B, etc.) you should not
> >> explicitly depend on A.
> >>
> >> On Thu, Jun 5, 2014 at 11:32 AM, Denis Gervalle <[hidden email]> wrote:
> >> > Hi devs,
> >> >
> >> > I am reviving this proposal since we never came to a conclusion,
> >> > and I
> >> have
> >> > the feeling that our deps become more and more convoluted.
> >> >
> >> > To resume what I get from past history with my own vision:
> >> >
> >> > 1) Since our modules are getting really modular, it should never
> silently
> >> > depends on transitive dependency of another module (with IMO some
> >> > exception, see 3). So any undeclared deps found by
> >> > dependency:analyse should be explicitly declare in the effective
> >> > pom (Vincent POV as
> well)
> >> > 2) Apply maven principle, we should reuse and apply
> >> > convention-over-configuration over configuration, so common
> >> > dependency like slf4j,
> >> xwiki-commons-stability
> >> > ? ... should be in a high level parent pom
> >> > 3) We may rely on some very tight transitive dependency, for
> >> > exemple,
> it
> >> > would be really curious that xwiki-commons-component-api stop
> providing
> >> > javax.inject, or that xwki-commons-test-components stop providing
> junit,
> >> > mockito, and al.
> >> >
> >> > It would be nice to add those rules in our best practice and to
> >> > always check our pom upon finishing changes in a module. The best
> >> > would be to enforce those rules, but this is not easy since
> >> > static analysis is
> >> limited
> >> > and could create false positive.
> >> >
> >> > WDYT ?
> >> >
> >> >
> >> > On Mon, Aug 15, 2011 at 10:31 PM, Thomas Mortagne <
> >> [hidden email]
> >> >> wrote:
> >> >
> >> >> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol
> >> >> <[hidden email]>
> >> >> wrote:
> >> >> >
> >> >> > On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
> >> >> >
> >> >> >> On 08/12/2011 07:50 AM, Vincent Massol wrote:
> >> >> >>> Hi devs,
> >> >> >>>
> >> >> >>> Running mvn dependency:dependency-analyze produces
> >> >> >>> interesting
> >> results.
> >> >> >>>
> >> >> >>> For example:
> >> >> >>>
> >> >> >>> [INFO]
> >> >>
> >>
> ----------------------------------------------------------------------
> ----------------------------------------------------------------------
> -------------------------------
> >> >> >>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
> >> >> >>> [INFO]
> >> >>
> >>
> ----------------------------------------------------------------------
> ----------------------------------------------------------------------
> -------------------------------
> >> >> >>> …
> >> >> >>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli)
> >> >> >>> @
> >> >> xwiki-commons-properties ---
> >> >> >>> [WARNING] Used undeclared dependencies found:
> >> >> >>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
> >> >> >>> [WARNING]    javax.inject:javax.inject:jar:1:compile
> >> >> >>> [WARNING] Unused declared dependencies found:
> >> >> >>> [WARNING]
> >> >>
>  
> org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
> >> >> >>> [WARNING]
> >>  org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
> >> >> >>> [WARNING]
>  org.hibernate:hibernate-validator:jar:4.2.0.Final:test
> >> >> >>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
> >> >> >>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
> >> >> >>>
> >> >> >>> The question is (for this module but more generally for all
> others):
> >> >> >>> * Should we add slf4j and javax.inject reps in the pom.xml
> >> >> >>> for
> this
> >> >> module? (for ex today slf4j and javax.inject are found in the
> >> component-api
> >> >> dep)
> >> >> >>>
> >> >> >>> I think we should, wdyt?
> >> >> >>
> >> >> >> +1 as well.
> >> >> >
> >> >> > hmm actually we need to decide about the following:
> >> >> >
> >> >> > * In order to simplify writing pom.xml for modules having
> components
> >> >> (i.e. depending on xwiki-commons-component-api) I had added the
> >> following
> >> >> to  xwiki-commons-component-api/pom.xml:
> >> >> >
> >> >> >    <!-- Make it easy for components that wish to log - They
> >> >> > don't
> have
> >> >> to explicitly import SLF4J -->
> >> >> >    <dependency>
> >> >> >      <groupId>org.slf4j</groupId>
> >> >> >      <artifactId>slf4j-api</artifactId>
> >> >> >    </dependency>
> >> >> >
> >> >> > * In the same manner we have a dependency on javax.inject in
> >> >> xwiki-commons-component-api/pom.xml:
> >> >> >
> >> >> >    <!-- We add this dependency here so that users of the
> >> >> > Component
> API
> >> >> just need to depend on this artifact and
> >> >> >         don't have to explicitly add a dependency on
> >> >> javax.inject:java.inject. -->
> >> >> >    <dependency>
> >> >> >      <groupId>javax.inject</groupId>
> >> >> >      <artifactId>javax.inject</artifactId>
> >> >> >      <version>1</version>
> >> >> >    </dependency>
> >> >> >
> >> >> > So the question is: do we want to force each module depending
> >> >> > on
> >> >> xwiki-commons-component-api to have to declare an explicit dep
> >> >> on javax.inject and org.slf4j?
> >> >> >
> >> >> > I'm not so sure about that…
> >> >>
> >> >> I'm -0 and near -1 to list this kind of dependencies, using
> >> >> slf4j or javax.inject are what you HAVE TO use when you write an
> >> >> XWiki component so it's redundant from my POV.
> >> >>
> >> >> >
> >> >> > WDYT?
> >> >> >
> >> >> > Thanks
> >> >> > -Vincent
> >> >> >
> >> >> >>> Note that the "Unused declared dependencies found:" doesn't
> always
> >> >> generate correct results as is the case here. This is mostly
> >> >> because
> >> it's a
> >> >> static byte code check so any dep used at runtime will be
> >> >> considered
> >> unused.
> >> >> >>> See
> >> >>
> >>
> http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dep
> endency-plugin.html
> >> >> >>
> >> >> >> Some of these dependencies are not used directly by us, but
> >> >> >> are
> >> needed
> >> >> >> transitively by another library. For example, slf4j needs
> >> >> >> logback,
> >> which
> >> >> >> we never use directly (although we don't really declare it in
> every
> >> >> >> module that does logging). Hibernate needs us to pick a
> >> >> >> cache, a connection pool, validator, and a bytecode manipulation utility.
> So
> >> yes,
> >> >> >> we can safely ignore most of these false negatives, but we
> >> >> >> should
> >> still
> >> >> >> try to remove those that are really wrongfully declared as
> >> dependencies.
> >> >> >>
> >> >> >>> Thanks
> >> >> >>> -Vincent
> >> >> >
> >> >> > _______________________________________________
> >> >> > devs mailing list
> >> >> > [hidden email]
> >> >> > http://lists.xwiki.org/mailman/listinfo/devs
> >> >> >
> >> >>
> >> >>
> >> >>
> >> >> --
> >> >> Thomas Mortagne
> >> >> _______________________________________________
> >> >> devs mailing list
> >> >> [hidden email]
> >> >> http://lists.xwiki.org/mailman/listinfo/devs
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Denis Gervalle
> >> > SOFTEC sa - CEO
> >> > _______________________________________________
> >> > devs mailing list
> >> > [hidden email]
> >> > http://lists.xwiki.org/mailman/listinfo/devs
> >>
> >>
> >>
> >> --
> >> Thomas Mortagne
> >> _______________________________________________
> >> devs mailing list
> >> [hidden email]
> >> http://lists.xwiki.org/mailman/listinfo/devs
> >>
> >
> >
> >
> > --
> > Denis Gervalle
> > SOFTEC sa - CEO
> > _______________________________________________
> > devs mailing list
> > [hidden email]
> > http://lists.xwiki.org/mailman/listinfo/devs
>
>
>
> --
> Thomas Mortagne
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs
>



--
Denis Gervalle
SOFTEC sa - CEO
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Thomas Mortagne
Administrator
In reply to this post by Denis Gervalle-2
On Thu, Jun 5, 2014 at 3:10 PM, Denis Gervalle <[hidden email]> wrote:

> On Thu, Jun 5, 2014 at 2:27 PM, Thomas Mortagne <[hidden email]>
> wrote:
>
>> On Thu, Jun 5, 2014 at 2:14 PM, Denis Gervalle <[hidden email]> wrote:
>> > What happen if you also use dependency A not just because of B ?
>>
>> You put a dependency on A.
>>
>
> But you may not see that so easily when you change a few line in an
> existing module. Nothing will complains until you remove your deps to B.

If all your are saying here is that we don't have tools to enforce
that rule then yes we don't have tool to enforce that rule but we
don't have any to apply any of the proposed rules so far.

The only tool we have is going to force you put a dependency on each
and every import you have. Do you really want to apply that ?

>
>
>>
>> > How do you determine "because of B" ?
>>
>> By thinking.
>>
>
> Ok, I rephrase my question.
> Could you define what you consider a usage of dependency A because of B and
> the opposite ?

I'm not going to copy/paste a dictionary definition of "because of".
It's pretty obvious to me why I'm using an API, if B is not in the
equation does my code still work ?

>
>
>>
>> >
>> > And what to you think of xwiki-commons-test-component that is a deps of
>> > xwiki-platform-core ?
>>
>> It's wrong IMO. Any forced dependency is wrong IMO.
>>
>> > Should we remove it ?
>>
>> Yes we should remove it.
>>
>
> Why do we get it ? Its removal could become an nightmare... but if we agree
> on that, we should remove it ASAP.

Yes if we agree on that we should remove it. You don't have such
forced dependency on commons and it does not make it a nightmare...

>
>
>>
>> > What about deps for logging ?
>>
>> Depends how you use it, the logger used with @Inject is an official
>> feature of our component framework so xwiki-commons-component-api
>> should be enough.
>>
>> > And could we add xwiki-commons-stability (probably provided scope) to a
>> > high level pom to avoid adding/removing it all the time ? (or forget it,
>> > since it come with xwiki-commons-component-api currently) ?
>>
>> It's far from being used everywhere and there is no rule forcing to
>> use it, you set @Unstable when an API is unstable, it's not forbidden
>> to not go through @Unstable. Plus you are supposed to remove that
>> annotation after some time.
>>
>
> Ok, so not deps at any scope in any high level poms. This seems opposite to
> what Sergiu proposed, but it would be nice to agree on a rule.

No forced dependency if it's possible any if the submodule won't need
it. It can make sense in some very specific use cases where all
submodules will have this dependency or else they have nothing to do
here (for example dependency on rendering-api in
xwiki-platform-rendering) but it's certainly not true for something
like root commons or platform pom.

>
> To sum up, currently I am not sure the exception rule "because of" is clear
> enough to not create confusion. I also agree with Sergiu that we should
> list all (no warning of used deps not declared in dependency:analysis),
> this make the rule clear at least. I am not against factoring common
> infrastructure in a single place, but Thomas seems to be clearly -1.
>
> It would be nice to have more feedback from other committers ! This is not
> a minor aspect of our best practice IMO.
>
>
>>
>> >
>> > WDYT ?
>> >
>> >
>> >
>> > On Thu, Jun 5, 2014 at 11:42 AM, Thomas Mortagne <
>> [hidden email]>
>> > wrote:
>> >
>> >> For me the rule to apply is simple: when you require dependency A
>> >> because of another dependency B (B expose A in it's API, you implement
>> >> an interface of A to be found by B, etc.) you should not explicitly
>> >> depend on A.
>> >>
>> >> On Thu, Jun 5, 2014 at 11:32 AM, Denis Gervalle <[hidden email]> wrote:
>> >> > Hi devs,
>> >> >
>> >> > I am reviving this proposal since we never came to a conclusion, and I
>> >> have
>> >> > the feeling that our deps become more and more convoluted.
>> >> >
>> >> > To resume what I get from past history with my own vision:
>> >> >
>> >> > 1) Since our modules are getting really modular, it should never
>> silently
>> >> > depends on transitive dependency of another module (with IMO some
>> >> > exception, see 3). So any undeclared deps found by dependency:analyse
>> >> > should be explicitly declare in the effective pom (Vincent POV as
>> well)
>> >> > 2) Apply maven principle, we should reuse and apply
>> >> > convention-over-configuration
>> >> > over configuration, so common dependency like slf4j,
>> >> xwiki-commons-stability
>> >> > ? ... should be in a high level parent pom
>> >> > 3) We may rely on some very tight transitive dependency, for exemple,
>> it
>> >> > would be really curious that xwiki-commons-component-api stop
>> providing
>> >> > javax.inject, or that xwki-commons-test-components stop providing
>> junit,
>> >> > mockito, and al.
>> >> >
>> >> > It would be nice to add those rules in our best practice and to always
>> >> > check our pom upon finishing changes in a module. The best would be to
>> >> > enforce those rules, but this is not easy since static analysis is
>> >> limited
>> >> > and could create false positive.
>> >> >
>> >> > WDYT ?
>> >> >
>> >> >
>> >> > On Mon, Aug 15, 2011 at 10:31 PM, Thomas Mortagne <
>> >> [hidden email]
>> >> >> wrote:
>> >> >
>> >> >> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <[hidden email]>
>> >> >> wrote:
>> >> >> >
>> >> >> > On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
>> >> >> >
>> >> >> >> On 08/12/2011 07:50 AM, Vincent Massol wrote:
>> >> >> >>> Hi devs,
>> >> >> >>>
>> >> >> >>> Running mvn dependency:dependency-analyze produces interesting
>> >> results.
>> >> >> >>>
>> >> >> >>> For example:
>> >> >> >>>
>> >> >> >>> [INFO]
>> >> >>
>> >>
>> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>> >> >> >>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
>> >> >> >>> [INFO]
>> >> >>
>> >>
>> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>> >> >> >>> …
>> >> >> >>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @
>> >> >> xwiki-commons-properties ---
>> >> >> >>> [WARNING] Used undeclared dependencies found:
>> >> >> >>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
>> >> >> >>> [WARNING]    javax.inject:javax.inject:jar:1:compile
>> >> >> >>> [WARNING] Unused declared dependencies found:
>> >> >> >>> [WARNING]
>> >> >>
>>  org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
>> >> >> >>> [WARNING]
>> >>  org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
>> >> >> >>> [WARNING]
>>  org.hibernate:hibernate-validator:jar:4.2.0.Final:test
>> >> >> >>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
>> >> >> >>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
>> >> >> >>>
>> >> >> >>> The question is (for this module but more generally for all
>> others):
>> >> >> >>> * Should we add slf4j and javax.inject reps in the pom.xml for
>> this
>> >> >> module? (for ex today slf4j and javax.inject are found in the
>> >> component-api
>> >> >> dep)
>> >> >> >>>
>> >> >> >>> I think we should, wdyt?
>> >> >> >>
>> >> >> >> +1 as well.
>> >> >> >
>> >> >> > hmm actually we need to decide about the following:
>> >> >> >
>> >> >> > * In order to simplify writing pom.xml for modules having
>> components
>> >> >> (i.e. depending on xwiki-commons-component-api) I had added the
>> >> following
>> >> >> to  xwiki-commons-component-api/pom.xml:
>> >> >> >
>> >> >> >    <!-- Make it easy for components that wish to log - They don't
>> have
>> >> >> to explicitly import SLF4J -->
>> >> >> >    <dependency>
>> >> >> >      <groupId>org.slf4j</groupId>
>> >> >> >      <artifactId>slf4j-api</artifactId>
>> >> >> >    </dependency>
>> >> >> >
>> >> >> > * In the same manner we have a dependency on javax.inject in
>> >> >> xwiki-commons-component-api/pom.xml:
>> >> >> >
>> >> >> >    <!-- We add this dependency here so that users of the Component
>> API
>> >> >> just need to depend on this artifact and
>> >> >> >         don't have to explicitly add a dependency on
>> >> >> javax.inject:java.inject. -->
>> >> >> >    <dependency>
>> >> >> >      <groupId>javax.inject</groupId>
>> >> >> >      <artifactId>javax.inject</artifactId>
>> >> >> >      <version>1</version>
>> >> >> >    </dependency>
>> >> >> >
>> >> >> > So the question is: do we want to force each module depending on
>> >> >> xwiki-commons-component-api to have to declare an explicit dep on
>> >> >> javax.inject and org.slf4j?
>> >> >> >
>> >> >> > I'm not so sure about that…
>> >> >>
>> >> >> I'm -0 and near -1 to list this kind of dependencies, using slf4j or
>> >> >> javax.inject are what you HAVE TO use when you write an XWiki
>> >> >> component so it's redundant from my POV.
>> >> >>
>> >> >> >
>> >> >> > WDYT?
>> >> >> >
>> >> >> > Thanks
>> >> >> > -Vincent
>> >> >> >
>> >> >> >>> Note that the "Unused declared dependencies found:" doesn't
>> always
>> >> >> generate correct results as is the case here. This is mostly because
>> >> it's a
>> >> >> static byte code check so any dep used at runtime will be considered
>> >> unused.
>> >> >> >>> See
>> >> >>
>> >>
>> http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
>> >> >> >>
>> >> >> >> Some of these dependencies are not used directly by us, but are
>> >> needed
>> >> >> >> transitively by another library. For example, slf4j needs logback,
>> >> which
>> >> >> >> we never use directly (although we don't really declare it in
>> every
>> >> >> >> module that does logging). Hibernate needs us to pick a cache, a
>> >> >> >> connection pool, validator, and a bytecode manipulation utility.
>> So
>> >> yes,
>> >> >> >> we can safely ignore most of these false negatives, but we should
>> >> still
>> >> >> >> try to remove those that are really wrongfully declared as
>> >> dependencies.
>> >> >> >>
>> >> >> >>> Thanks
>> >> >> >>> -Vincent
>> >> >> >
>> >> >> > _______________________________________________
>> >> >> > devs mailing list
>> >> >> > [hidden email]
>> >> >> > http://lists.xwiki.org/mailman/listinfo/devs
>> >> >> >
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Thomas Mortagne
>> >> >> _______________________________________________
>> >> >> devs mailing list
>> >> >> [hidden email]
>> >> >> http://lists.xwiki.org/mailman/listinfo/devs
>> >> >>
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Denis Gervalle
>> >> > SOFTEC sa - CEO
>> >> > _______________________________________________
>> >> > devs mailing list
>> >> > [hidden email]
>> >> > http://lists.xwiki.org/mailman/listinfo/devs
>> >>
>> >>
>> >>
>> >> --
>> >> Thomas Mortagne
>> >> _______________________________________________
>> >> devs mailing list
>> >> [hidden email]
>> >> http://lists.xwiki.org/mailman/listinfo/devs
>> >>
>> >
>> >
>> >
>> > --
>> > Denis Gervalle
>> > SOFTEC sa - CEO
>> > _______________________________________________
>> > devs mailing list
>> > [hidden email]
>> > http://lists.xwiki.org/mailman/listinfo/devs
>>
>>
>>
>> --
>> Thomas Mortagne
>> _______________________________________________
>> devs mailing list
>> [hidden email]
>> http://lists.xwiki.org/mailman/listinfo/devs
>>
>
>
>
> --
> Denis Gervalle
> SOFTEC sa - CEO
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs



--
Thomas Mortagne
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

jerem
In reply to this post by Denis Gervalle-2
Hi,


2014-06-05 15:10 GMT+02:00 Denis Gervalle <[hidden email]>:

> On Thu, Jun 5, 2014 at 2:27 PM, Thomas Mortagne <[hidden email]
> >
> wrote:
>
> > On Thu, Jun 5, 2014 at 2:14 PM, Denis Gervalle <[hidden email]> wrote:
> > > What happen if you also use dependency A not just because of B ?
> >
> > You put a dependency on A.
> >
>
> But you may not see that so easily when you change a few line in an
> existing module. Nothing will complains until you remove your deps to B.
>
>
> >
> > > How do you determine "because of B" ?
> >
> > By thinking.
> >
>
> Ok, I rephrase my question.
> Could you define what you consider a usage of dependency A because of B and
> the opposite ?
>
>
If I understood, you depend on B, and by transitivity, you also depend on A
(indirectly).
Then, from your module you use some classes of A (brought to you
indirectly, so "magically", it compiles).
So you never explicitly declare the dep on A from your module. If one day
you remove dependency on B for any reason, you will also loose A (and also
you can say it's an explicit dependency that is not "explicit", though it's
a matter of taste or point of view to consider transitive deps not to be
explicit ...).

The opposite would be to declare explicit dep on A from your module, while
you don't need it at all - only B needs it.



>
> >
> > >
> > > And what to you think of xwiki-commons-test-component that is a deps of
> > > xwiki-platform-core ?
> >
> > It's wrong IMO. Any forced dependency is wrong IMO.
> >
> > > Should we remove it ?
> >
> > Yes we should remove it.
> >
>
> Why do we get it ? Its removal could become an nightmare... but if we agree
> on that, we should remove it ASAP.
>
>
> >
> > > What about deps for logging ?
> >
> > Depends how you use it, the logger used with @Inject is an official
> > feature of our component framework so xwiki-commons-component-api
> > should be enough.
> >
> > > And could we add xwiki-commons-stability (probably provided scope) to a
> > > high level pom to avoid adding/removing it all the time ? (or forget
> it,
> > > since it come with xwiki-commons-component-api currently) ?
> >
> > It's far from being used everywhere and there is no rule forcing to
> > use it, you set @Unstable when an API is unstable, it's not forbidden
> > to not go through @Unstable. Plus you are supposed to remove that
> > annotation after some time.
> >
>
> Ok, so not deps at any scope in any high level poms. This seems opposite to
> what Sergiu proposed, but it would be nice to agree on a rule.
>
> To sum up, currently I am not sure the exception rule "because of" is clear
> enough to not create confusion. I also agree with Sergiu that we should
> list all (no warning of used deps not declared in dependency:analysis),
> this make the rule clear at least. I am not against factoring common
> infrastructure in a single place, but Thomas seems to be clearly -1.
>

I'm not sure it relates to what you describe above. Factoring deps in a
common infrastructure is more a matter of using dependencyManagement at
correct level (top-most usually), but the rule to declare only deps that
are really needed per module is a very widely used best practice.

From a contributor (not committer) point of view, it's sometimes annoying
to have to update your builds because of this kind of changes. Obviously it
would be difficult to consider those dependency graphs as APIs :) , but at
the same time when they evolve too frequently, it can be considered painful
to follow by devs in general ...
Some are not impacting, but for example regarding @Unstable, if it's not
brought anymore by xwiki-commons-component-api, it will break my build when
I upgrade xwiki versions.

With Maven, it's not really a "bad" practice, to consider that some poms
are used mainly to bring to you a set of logically related dependencies by
transitivity (there's even the "import" scope, though it's not the best
sample of good practice I agree). Question is more (case by case), is it
really "bad" to bring @Unstable by default to everyone that develops an
xwiki component, even if he will never use it / don't use it anymore ?

>
>
> It would be nice to have more feedback from other committers ! This is not
> a minor aspect of our best practice IMO.
>
>
> >
> > >
> > > WDYT ?
> > >
> > >
> > >
> > > On Thu, Jun 5, 2014 at 11:42 AM, Thomas Mortagne <
> > [hidden email]>
> > > wrote:
> > >
> > >> For me the rule to apply is simple: when you require dependency A
> > >> because of another dependency B (B expose A in it's API, you implement
> > >> an interface of A to be found by B, etc.) you should not explicitly
> > >> depend on A.
> > >>
> > >> On Thu, Jun 5, 2014 at 11:32 AM, Denis Gervalle <[hidden email]>
> wrote:
> > >> > Hi devs,
> > >> >
> > >> > I am reviving this proposal since we never came to a conclusion,
> and I
> > >> have
> > >> > the feeling that our deps become more and more convoluted.
> > >> >
> > >> > To resume what I get from past history with my own vision:
> > >> >
> > >> > 1) Since our modules are getting really modular, it should never
> > silently
> > >> > depends on transitive dependency of another module (with IMO some
> > >> > exception, see 3). So any undeclared deps found by
> dependency:analyse
> > >> > should be explicitly declare in the effective pom (Vincent POV as
> > well)
> > >> > 2) Apply maven principle, we should reuse and apply
> > >> > convention-over-configuration
> > >> > over configuration, so common dependency like slf4j,
> > >> xwiki-commons-stability
> > >> > ? ... should be in a high level parent pom
> > >> > 3) We may rely on some very tight transitive dependency, for
> exemple,
> > it
> > >> > would be really curious that xwiki-commons-component-api stop
> > providing
> > >> > javax.inject, or that xwki-commons-test-components stop providing
> > junit,
> > >> > mockito, and al.
> > >> >
> > >> > It would be nice to add those rules in our best practice and to
> always
> > >> > check our pom upon finishing changes in a module. The best would be
> to
> > >> > enforce those rules, but this is not easy since static analysis is
> > >> limited
> > >> > and could create false positive.
> > >> >
> > >> > WDYT ?
> > >> >
> > >> >
> > >> > On Mon, Aug 15, 2011 at 10:31 PM, Thomas Mortagne <
> > >> [hidden email]
> > >> >> wrote:
> > >> >
> > >> >> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <
> [hidden email]>
> > >> >> wrote:
> > >> >> >
> > >> >> > On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
> > >> >> >
> > >> >> >> On 08/12/2011 07:50 AM, Vincent Massol wrote:
> > >> >> >>> Hi devs,
> > >> >> >>>
> > >> >> >>> Running mvn dependency:dependency-analyze produces interesting
> > >> results.
> > >> >> >>>
> > >> >> >>> For example:
> > >> >> >>>
> > >> >> >>> [INFO]
> > >> >>
> > >>
> >
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> > >> >> >>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
> > >> >> >>> [INFO]
> > >> >>
> > >>
> >
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> > >> >> >>> …
> > >> >> >>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @
> > >> >> xwiki-commons-properties ---
> > >> >> >>> [WARNING] Used undeclared dependencies found:
> > >> >> >>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
> > >> >> >>> [WARNING]    javax.inject:javax.inject:jar:1:compile
> > >> >> >>> [WARNING] Unused declared dependencies found:
> > >> >> >>> [WARNING]
> > >> >>
> >  org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
> > >> >> >>> [WARNING]
> > >>  org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
> > >> >> >>> [WARNING]
> >  org.hibernate:hibernate-validator:jar:4.2.0.Final:test
> > >> >> >>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
> > >> >> >>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
> > >> >> >>>
> > >> >> >>> The question is (for this module but more generally for all
> > others):
> > >> >> >>> * Should we add slf4j and javax.inject reps in the pom.xml for
> > this
> > >> >> module? (for ex today slf4j and javax.inject are found in the
> > >> component-api
> > >> >> dep)
> > >> >> >>>
> > >> >> >>> I think we should, wdyt?
> > >> >> >>
> > >> >> >> +1 as well.
> > >> >> >
> > >> >> > hmm actually we need to decide about the following:
> > >> >> >
> > >> >> > * In order to simplify writing pom.xml for modules having
> > components
> > >> >> (i.e. depending on xwiki-commons-component-api) I had added the
> > >> following
> > >> >> to  xwiki-commons-component-api/pom.xml:
> > >> >> >
> > >> >> >    <!-- Make it easy for components that wish to log - They don't
> > have
> > >> >> to explicitly import SLF4J -->
> > >> >> >    <dependency>
> > >> >> >      <groupId>org.slf4j</groupId>
> > >> >> >      <artifactId>slf4j-api</artifactId>
> > >> >> >    </dependency>
> > >> >> >
> > >> >> > * In the same manner we have a dependency on javax.inject in
> > >> >> xwiki-commons-component-api/pom.xml:
> > >> >> >
> > >> >> >    <!-- We add this dependency here so that users of the
> Component
> > API
> > >> >> just need to depend on this artifact and
> > >> >> >         don't have to explicitly add a dependency on
> > >> >> javax.inject:java.inject. -->
> > >> >> >    <dependency>
> > >> >> >      <groupId>javax.inject</groupId>
> > >> >> >      <artifactId>javax.inject</artifactId>
> > >> >> >      <version>1</version>
> > >> >> >    </dependency>
> > >> >> >
> > >> >> > So the question is: do we want to force each module depending on
> > >> >> xwiki-commons-component-api to have to declare an explicit dep on
> > >> >> javax.inject and org.slf4j?
> > >> >> >
> > >> >> > I'm not so sure about that…
> > >> >>
> > >> >> I'm -0 and near -1 to list this kind of dependencies, using slf4j
> or
> > >> >> javax.inject are what you HAVE TO use when you write an XWiki
> > >> >> component so it's redundant from my POV.
> > >> >>
> > >> >> >
> > >> >> > WDYT?
> > >> >> >
> > >> >> > Thanks
> > >> >> > -Vincent
> > >> >> >
> > >> >> >>> Note that the "Unused declared dependencies found:" doesn't
> > always
> > >> >> generate correct results as is the case here. This is mostly
> because
> > >> it's a
> > >> >> static byte code check so any dep used at runtime will be
> considered
> > >> unused.
> > >> >> >>> See
> > >> >>
> > >>
> >
> http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
> > >> >> >>
> > >> >> >> Some of these dependencies are not used directly by us, but are
> > >> needed
> > >> >> >> transitively by another library. For example, slf4j needs
> logback,
> > >> which
> > >> >> >> we never use directly (although we don't really declare it in
> > every
> > >> >> >> module that does logging). Hibernate needs us to pick a cache, a
> > >> >> >> connection pool, validator, and a bytecode manipulation utility.
> > So
> > >> yes,
> > >> >> >> we can safely ignore most of these false negatives, but we
> should
> > >> still
> > >> >> >> try to remove those that are really wrongfully declared as
> > >> dependencies.
> > >> >> >>
> > >> >> >>> Thanks
> > >> >> >>> -Vincent
> > >> >> >
> > >> >> > _______________________________________________
> > >> >> > devs mailing list
> > >> >> > [hidden email]
> > >> >> > http://lists.xwiki.org/mailman/listinfo/devs
> > >> >> >
> > >> >>
> > >> >>
> > >> >>
> > >> >> --
> > >> >> Thomas Mortagne
> > >> >> _______________________________________________
> > >> >> devs mailing list
> > >> >> [hidden email]
> > >> >> http://lists.xwiki.org/mailman/listinfo/devs
> > >> >>
> > >> >
> > >> >
> > >> >
> > >> > --
> > >> > Denis Gervalle
> > >> > SOFTEC sa - CEO
> > >> > _______________________________________________
> > >> > devs mailing list
> > >> > [hidden email]
> > >> > http://lists.xwiki.org/mailman/listinfo/devs
> > >>
> > >>
> > >>
> > >> --
> > >> Thomas Mortagne
> > >> _______________________________________________
> > >> devs mailing list
> > >> [hidden email]
> > >> http://lists.xwiki.org/mailman/listinfo/devs
> > >>
> > >
> > >
> > >
> > > --
> > > Denis Gervalle
> > > SOFTEC sa - CEO
> > > _______________________________________________
> > > devs mailing list
> > > [hidden email]
> > > http://lists.xwiki.org/mailman/listinfo/devs
> >
> >
> >
> > --
> > Thomas Mortagne
> > _______________________________________________
> > devs mailing list
> > [hidden email]
> > http://lists.xwiki.org/mailman/listinfo/devs
> >
>
>
>
> --
> Denis Gervalle
> SOFTEC sa - CEO
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs
>
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Denis Gervalle-2
In reply to this post by Thomas Mortagne
On Thu, Jun 5, 2014 at 3:42 PM, Thomas Mortagne <[hidden email]>
wrote:

> On Thu, Jun 5, 2014 at 3:10 PM, Denis Gervalle <[hidden email]> wrote:
> > On Thu, Jun 5, 2014 at 2:27 PM, Thomas Mortagne <
> [hidden email]>
> > wrote:
> >
> >> On Thu, Jun 5, 2014 at 2:14 PM, Denis Gervalle <[hidden email]> wrote:
> >> > What happen if you also use dependency A not just because of B ?
> >>
> >> You put a dependency on A.
> >>
> >
> > But you may not see that so easily when you change a few line in an
> > existing module. Nothing will complains until you remove your deps to B.
>
> If all your are saying here is that we don't have tools to enforce
> that rule then yes we don't have tool to enforce that rule but we
> don't have any to apply any of the proposed rules so far.
>
> The only tool we have is going to force you put a dependency on each
> and every import you have. Do you really want to apply that ?
>

If this is the best way to have a consistent usage of dependencies and to
keep it up to date, why not ?


>
> >
> >
> >>
> >> > How do you determine "because of B" ?
> >>
> >> By thinking.
> >>
> >
> > Ok, I rephrase my question.
> > Could you define what you consider a usage of dependency A because of B
> and
> > the opposite ?
>
> I'm not going to copy/paste a dictionary definition of "because of".
>

If you cannot define the rule, you can apply it, that's all.


> It's pretty obvious to me why I'm using an API, if B is not in the
> equation does my code still work ?
>

It is not that obvious, your code is not a monolith.


>
> >
> >
> >>
> >> >
> >> > And what to you think of xwiki-commons-test-component that is a deps
> of
> >> > xwiki-platform-core ?
> >>
> >> It's wrong IMO. Any forced dependency is wrong IMO.
> >>
> >> > Should we remove it ?
> >>
> >> Yes we should remove it.
> >>
> >
> > Why do we get it ? Its removal could become an nightmare... but if we
> agree
> > on that, we should remove it ASAP.
>
> Yes if we agree on that we should remove it. You don't have such
> forced dependency on commons and it does not make it a nightmare...
>

You missed my point, removing it now could become a nightmare, and keeping
it further surely increase it. I completely agree that if we do not want
any useless deps in our module, we should not add such "global" deps.


>
> >
> >
> >>
> >> > What about deps for logging ?
> >>
> >> Depends how you use it, the logger used with @Inject is an official
> >> feature of our component framework so xwiki-commons-component-api
> >> should be enough.
> >>
> >> > And could we add xwiki-commons-stability (probably provided scope) to
> a
> >> > high level pom to avoid adding/removing it all the time ? (or forget
> it,
> >> > since it come with xwiki-commons-component-api currently) ?
> >>
> >> It's far from being used everywhere and there is no rule forcing to
> >> use it, you set @Unstable when an API is unstable, it's not forbidden
> >> to not go through @Unstable. Plus you are supposed to remove that
> >> annotation after some time.
> >>
> >
> > Ok, so not deps at any scope in any high level poms. This seems opposite
> to
> > what Sergiu proposed, but it would be nice to agree on a rule.
>
> No forced dependency if it's possible any if the submodule won't need
> it. It can make sense in some very specific use cases where all
> submodules will have this dependency or else they have nothing to do
> here (for example dependency on rendering-api in
> xwiki-platform-rendering) but it's certainly not true for something
> like root commons or platform pom.
>
> >
> > To sum up, currently I am not sure the exception rule "because of" is
> clear
> > enough to not create confusion. I also agree with Sergiu that we should
> > list all (no warning of used deps not declared in dependency:analysis),
> > this make the rule clear at least. I am not against factoring common
> > infrastructure in a single place, but Thomas seems to be clearly -1.
> >
> > It would be nice to have more feedback from other committers ! This is
> not
> > a minor aspect of our best practice IMO.
> >
> >
> >>
> >> >
> >> > WDYT ?
> >> >
> >> >
> >> >
> >> > On Thu, Jun 5, 2014 at 11:42 AM, Thomas Mortagne <
> >> [hidden email]>
> >> > wrote:
> >> >
> >> >> For me the rule to apply is simple: when you require dependency A
> >> >> because of another dependency B (B expose A in it's API, you
> implement
> >> >> an interface of A to be found by B, etc.) you should not explicitly
> >> >> depend on A.
> >> >>
> >> >> On Thu, Jun 5, 2014 at 11:32 AM, Denis Gervalle <[hidden email]>
> wrote:
> >> >> > Hi devs,
> >> >> >
> >> >> > I am reviving this proposal since we never came to a conclusion,
> and I
> >> >> have
> >> >> > the feeling that our deps become more and more convoluted.
> >> >> >
> >> >> > To resume what I get from past history with my own vision:
> >> >> >
> >> >> > 1) Since our modules are getting really modular, it should never
> >> silently
> >> >> > depends on transitive dependency of another module (with IMO some
> >> >> > exception, see 3). So any undeclared deps found by
> dependency:analyse
> >> >> > should be explicitly declare in the effective pom (Vincent POV as
> >> well)
> >> >> > 2) Apply maven principle, we should reuse and apply
> >> >> > convention-over-configuration
> >> >> > over configuration, so common dependency like slf4j,
> >> >> xwiki-commons-stability
> >> >> > ? ... should be in a high level parent pom
> >> >> > 3) We may rely on some very tight transitive dependency, for
> exemple,
> >> it
> >> >> > would be really curious that xwiki-commons-component-api stop
> >> providing
> >> >> > javax.inject, or that xwki-commons-test-components stop providing
> >> junit,
> >> >> > mockito, and al.
> >> >> >
> >> >> > It would be nice to add those rules in our best practice and to
> always
> >> >> > check our pom upon finishing changes in a module. The best would
> be to
> >> >> > enforce those rules, but this is not easy since static analysis is
> >> >> limited
> >> >> > and could create false positive.
> >> >> >
> >> >> > WDYT ?
> >> >> >
> >> >> >
> >> >> > On Mon, Aug 15, 2011 at 10:31 PM, Thomas Mortagne <
> >> >> [hidden email]
> >> >> >> wrote:
> >> >> >
> >> >> >> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <
> [hidden email]>
> >> >> >> wrote:
> >> >> >> >
> >> >> >> > On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
> >> >> >> >
> >> >> >> >> On 08/12/2011 07:50 AM, Vincent Massol wrote:
> >> >> >> >>> Hi devs,
> >> >> >> >>>
> >> >> >> >>> Running mvn dependency:dependency-analyze produces interesting
> >> >> results.
> >> >> >> >>>
> >> >> >> >>> For example:
> >> >> >> >>>
> >> >> >> >>> [INFO]
> >> >> >>
> >> >>
> >>
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> >> >> >> >>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
> >> >> >> >>> [INFO]
> >> >> >>
> >> >>
> >>
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> >> >> >> >>> …
> >> >> >> >>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @
> >> >> >> xwiki-commons-properties ---
> >> >> >> >>> [WARNING] Used undeclared dependencies found:
> >> >> >> >>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
> >> >> >> >>> [WARNING]    javax.inject:javax.inject:jar:1:compile
> >> >> >> >>> [WARNING] Unused declared dependencies found:
> >> >> >> >>> [WARNING]
> >> >> >>
> >>  org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
> >> >> >> >>> [WARNING]
> >> >>  org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
> >> >> >> >>> [WARNING]
> >>  org.hibernate:hibernate-validator:jar:4.2.0.Final:test
> >> >> >> >>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
> >> >> >> >>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
> >> >> >> >>>
> >> >> >> >>> The question is (for this module but more generally for all
> >> others):
> >> >> >> >>> * Should we add slf4j and javax.inject reps in the pom.xml for
> >> this
> >> >> >> module? (for ex today slf4j and javax.inject are found in the
> >> >> component-api
> >> >> >> dep)
> >> >> >> >>>
> >> >> >> >>> I think we should, wdyt?
> >> >> >> >>
> >> >> >> >> +1 as well.
> >> >> >> >
> >> >> >> > hmm actually we need to decide about the following:
> >> >> >> >
> >> >> >> > * In order to simplify writing pom.xml for modules having
> >> components
> >> >> >> (i.e. depending on xwiki-commons-component-api) I had added the
> >> >> following
> >> >> >> to  xwiki-commons-component-api/pom.xml:
> >> >> >> >
> >> >> >> >    <!-- Make it easy for components that wish to log - They
> don't
> >> have
> >> >> >> to explicitly import SLF4J -->
> >> >> >> >    <dependency>
> >> >> >> >      <groupId>org.slf4j</groupId>
> >> >> >> >      <artifactId>slf4j-api</artifactId>
> >> >> >> >    </dependency>
> >> >> >> >
> >> >> >> > * In the same manner we have a dependency on javax.inject in
> >> >> >> xwiki-commons-component-api/pom.xml:
> >> >> >> >
> >> >> >> >    <!-- We add this dependency here so that users of the
> Component
> >> API
> >> >> >> just need to depend on this artifact and
> >> >> >> >         don't have to explicitly add a dependency on
> >> >> >> javax.inject:java.inject. -->
> >> >> >> >    <dependency>
> >> >> >> >      <groupId>javax.inject</groupId>
> >> >> >> >      <artifactId>javax.inject</artifactId>
> >> >> >> >      <version>1</version>
> >> >> >> >    </dependency>
> >> >> >> >
> >> >> >> > So the question is: do we want to force each module depending on
> >> >> >> xwiki-commons-component-api to have to declare an explicit dep on
> >> >> >> javax.inject and org.slf4j?
> >> >> >> >
> >> >> >> > I'm not so sure about that…
> >> >> >>
> >> >> >> I'm -0 and near -1 to list this kind of dependencies, using slf4j
> or
> >> >> >> javax.inject are what you HAVE TO use when you write an XWiki
> >> >> >> component so it's redundant from my POV.
> >> >> >>
> >> >> >> >
> >> >> >> > WDYT?
> >> >> >> >
> >> >> >> > Thanks
> >> >> >> > -Vincent
> >> >> >> >
> >> >> >> >>> Note that the "Unused declared dependencies found:" doesn't
> >> always
> >> >> >> generate correct results as is the case here. This is mostly
> because
> >> >> it's a
> >> >> >> static byte code check so any dep used at runtime will be
> considered
> >> >> unused.
> >> >> >> >>> See
> >> >> >>
> >> >>
> >>
> http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
> >> >> >> >>
> >> >> >> >> Some of these dependencies are not used directly by us, but are
> >> >> needed
> >> >> >> >> transitively by another library. For example, slf4j needs
> logback,
> >> >> which
> >> >> >> >> we never use directly (although we don't really declare it in
> >> every
> >> >> >> >> module that does logging). Hibernate needs us to pick a cache,
> a
> >> >> >> >> connection pool, validator, and a bytecode manipulation
> utility.
> >> So
> >> >> yes,
> >> >> >> >> we can safely ignore most of these false negatives, but we
> should
> >> >> still
> >> >> >> >> try to remove those that are really wrongfully declared as
> >> >> dependencies.
> >> >> >> >>
> >> >> >> >>> Thanks
> >> >> >> >>> -Vincent
> >> >> >> >
> >> >> >> > _______________________________________________
> >> >> >> > devs mailing list
> >> >> >> > [hidden email]
> >> >> >> > http://lists.xwiki.org/mailman/listinfo/devs
> >> >> >> >
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >> --
> >> >> >> Thomas Mortagne
> >> >> >> _______________________________________________
> >> >> >> devs mailing list
> >> >> >> [hidden email]
> >> >> >> http://lists.xwiki.org/mailman/listinfo/devs
> >> >> >>
> >> >> >
> >> >> >
> >> >> >
> >> >> > --
> >> >> > Denis Gervalle
> >> >> > SOFTEC sa - CEO
> >> >> > _______________________________________________
> >> >> > devs mailing list
> >> >> > [hidden email]
> >> >> > http://lists.xwiki.org/mailman/listinfo/devs
> >> >>
> >> >>
> >> >>
> >> >> --
> >> >> Thomas Mortagne
> >> >> _______________________________________________
> >> >> devs mailing list
> >> >> [hidden email]
> >> >> http://lists.xwiki.org/mailman/listinfo/devs
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Denis Gervalle
> >> > SOFTEC sa - CEO
> >> > _______________________________________________
> >> > devs mailing list
> >> > [hidden email]
> >> > http://lists.xwiki.org/mailman/listinfo/devs
> >>
> >>
> >>
> >> --
> >> Thomas Mortagne
> >> _______________________________________________
> >> devs mailing list
> >> [hidden email]
> >> http://lists.xwiki.org/mailman/listinfo/devs
> >>
> >
> >
> >
> > --
> > Denis Gervalle
> > SOFTEC sa - CEO
> > _______________________________________________
> > devs mailing list
> > [hidden email]
> > http://lists.xwiki.org/mailman/listinfo/devs
>
>
>
> --
> Thomas Mortagne
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs
>



--
Denis Gervalle
SOFTEC sa - CEO
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Sergiu Dumitriu-2
In reply to this post by Roman Muntyanu
On 06/05/2014 09:23 AM, Roman Muntyanu wrote:

>>> Ok, I rephrase my question.
>>> Could you define what you consider a usage of dependency A because of B and the opposite?
>
> I'll dare explaining
>
> Indirect dependency (no dependency on A should be defined in C):
> C ----> B ----> A
>   * C uses classes from B
>   * B uses classes from A
>   * C does NOT use classes from A

This will NOT be reported by dependency:analyze, so this shouldn't be
declared as a dependency of C, ever.

> Direct dependency (explicit dependency on A should be defined in C)
> C ----> B ----> A
>  \_ _ _ _ _ _ _^
>   * C uses classes from B
>   * B uses classes from A
>   * C uses classes from A

This is actually the topic of the discussion. It's not about what B
uses, but what C uses.

Since C uses A, it's obvious that it should declare it as a dependency,
and not assume that B will be there to provide it, unless B is a
"grouping" component, like xwiki-commons-test-components, which is
supposed to bring everything needed for testing components.


Big +1 for declaring all the used dependencies, except for "boilerplate"
ones.

+1 for not declaring the transitive dependencies of standard groups:
testing tools chained to the commons-test-components, component modules
chained to xwiki-commons-component-api

-1 for declaring global dependencies, they're actually hidden inter-pom
dependencies.

--
Sergiu Dumitriu
http://purl.org/net/sergiu
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

Denis Gervalle-2
In reply to this post by jerem
On Thu, Jun 5, 2014 at 3:51 PM, Jeremie BOUSQUET <[hidden email]
> wrote:

> Hi,
>
>
> 2014-06-05 15:10 GMT+02:00 Denis Gervalle <[hidden email]>:
>
> > On Thu, Jun 5, 2014 at 2:27 PM, Thomas Mortagne <
> [hidden email]
> > >
> > wrote:
> >
> > > On Thu, Jun 5, 2014 at 2:14 PM, Denis Gervalle <[hidden email]> wrote:
> > > > What happen if you also use dependency A not just because of B ?
> > >
> > > You put a dependency on A.
> > >
> >
> > But you may not see that so easily when you change a few line in an
> > existing module. Nothing will complains until you remove your deps to B.
> >
> >
> > >
> > > > How do you determine "because of B" ?
> > >
> > > By thinking.
> > >
> >
> > Ok, I rephrase my question.
> > Could you define what you consider a usage of dependency A because of B
> and
> > the opposite ?
> >
> >
> If I understood, you depend on B, and by transitivity, you also depend on A
> (indirectly).
> Then, from your module you use some classes of A (brought to you
> indirectly, so "magically", it compiles).
> So you never explicitly declare the dep on A from your module. If one day
> you remove dependency on B for any reason, you will also loose A (and also
> you can say it's an explicit dependency that is not "explicit", though it's
> a matter of taste or point of view to consider transitive deps not to be
> explicit ...).
>
> The opposite would be to declare explicit dep on A from your module, while
> you don't need it at all - only B needs it.
>

You get it fully. The question is, could we define in a few word when the
transitivity could be used, because the usage of A is just "because of" B ?
Else, we should add a deps on A as soon as we import any class from A IMO.


>
>
>
> >
> > >
> > > >
> > > > And what to you think of xwiki-commons-test-component that is a deps
> of
> > > > xwiki-platform-core ?
> > >
> > > It's wrong IMO. Any forced dependency is wrong IMO.
> > >
> > > > Should we remove it ?
> > >
> > > Yes we should remove it.
> > >
> >
> > Why do we get it ? Its removal could become an nightmare... but if we
> agree
> > on that, we should remove it ASAP.
> >
> >
> > >
> > > > What about deps for logging ?
> > >
> > > Depends how you use it, the logger used with @Inject is an official
> > > feature of our component framework so xwiki-commons-component-api
> > > should be enough.
> > >
> > > > And could we add xwiki-commons-stability (probably provided scope)
> to a
> > > > high level pom to avoid adding/removing it all the time ? (or forget
> > it,
> > > > since it come with xwiki-commons-component-api currently) ?
> > >
> > > It's far from being used everywhere and there is no rule forcing to
> > > use it, you set @Unstable when an API is unstable, it's not forbidden
> > > to not go through @Unstable. Plus you are supposed to remove that
> > > annotation after some time.
> > >
> >
> > Ok, so not deps at any scope in any high level poms. This seems opposite
> to
> > what Sergiu proposed, but it would be nice to agree on a rule.
> >
> > To sum up, currently I am not sure the exception rule "because of" is
> clear
> > enough to not create confusion. I also agree with Sergiu that we should
> > list all (no warning of used deps not declared in dependency:analysis),
> > this make the rule clear at least. I am not against factoring common
> > infrastructure in a single place, but Thomas seems to be clearly -1.
> >
>
> I'm not sure it relates to what you describe above. Factoring deps in a
> common infrastructure is more a matter of using dependencyManagement at
> correct level (top-most usually), but the rule to declare only deps that
> are really needed per module is a very widely used best practice.
>
> From a contributor (not committer) point of view, it's sometimes annoying
> to have to update your builds because of this kind of changes. Obviously it
> would be difficult to consider those dependency graphs as APIs :) , but at
> the same time when they evolve too frequently, it can be considered painful
> to follow by devs in general ...
> Some are not impacting, but for example regarding @Unstable, if it's not
> brought anymore by xwiki-commons-component-api, it will break my build when
> I upgrade xwiki versions.
>

This is bad, since I do not see any reason for xwiki-commons-component-api
to bring xwiki-commons-stability to you.


>
> With Maven, it's not really a "bad" practice, to consider that some poms
> are used mainly to bring to you a set of logically related dependencies by
> transitivity (there's even the "import" scope, though it's not the best
> sample of good practice I agree). Question is more (case by case), is it
> really "bad" to bring @Unstable by default to everyone that develops an
> xwiki component, even if he will never use it / don't use it anymore ?
>

I would be positive, especially because it is not a runtime deps, and we
can simply use the "provided" scope.
Thomas do not seems to agree.

All I really want at the end, is that we have clear rule.
Unless we can define what "because of" is, the only rule I see valid
currently, would be to explicitly depends on any module we do an import
from.


>
> >
> >
> > It would be nice to have more feedback from other committers ! This is
> not
> > a minor aspect of our best practice IMO.
> >
> >
> > >
> > > >
> > > > WDYT ?
> > > >
> > > >
> > > >
> > > > On Thu, Jun 5, 2014 at 11:42 AM, Thomas Mortagne <
> > > [hidden email]>
> > > > wrote:
> > > >
> > > >> For me the rule to apply is simple: when you require dependency A
> > > >> because of another dependency B (B expose A in it's API, you
> implement
> > > >> an interface of A to be found by B, etc.) you should not explicitly
> > > >> depend on A.
> > > >>
> > > >> On Thu, Jun 5, 2014 at 11:32 AM, Denis Gervalle <[hidden email]>
> > wrote:
> > > >> > Hi devs,
> > > >> >
> > > >> > I am reviving this proposal since we never came to a conclusion,
> > and I
> > > >> have
> > > >> > the feeling that our deps become more and more convoluted.
> > > >> >
> > > >> > To resume what I get from past history with my own vision:
> > > >> >
> > > >> > 1) Since our modules are getting really modular, it should never
> > > silently
> > > >> > depends on transitive dependency of another module (with IMO some
> > > >> > exception, see 3). So any undeclared deps found by
> > dependency:analyse
> > > >> > should be explicitly declare in the effective pom (Vincent POV as
> > > well)
> > > >> > 2) Apply maven principle, we should reuse and apply
> > > >> > convention-over-configuration
> > > >> > over configuration, so common dependency like slf4j,
> > > >> xwiki-commons-stability
> > > >> > ? ... should be in a high level parent pom
> > > >> > 3) We may rely on some very tight transitive dependency, for
> > exemple,
> > > it
> > > >> > would be really curious that xwiki-commons-component-api stop
> > > providing
> > > >> > javax.inject, or that xwki-commons-test-components stop providing
> > > junit,
> > > >> > mockito, and al.
> > > >> >
> > > >> > It would be nice to add those rules in our best practice and to
> > always
> > > >> > check our pom upon finishing changes in a module. The best would
> be
> > to
> > > >> > enforce those rules, but this is not easy since static analysis is
> > > >> limited
> > > >> > and could create false positive.
> > > >> >
> > > >> > WDYT ?
> > > >> >
> > > >> >
> > > >> > On Mon, Aug 15, 2011 at 10:31 PM, Thomas Mortagne <
> > > >> [hidden email]
> > > >> >> wrote:
> > > >> >
> > > >> >> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <
> > [hidden email]>
> > > >> >> wrote:
> > > >> >> >
> > > >> >> > On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
> > > >> >> >
> > > >> >> >> On 08/12/2011 07:50 AM, Vincent Massol wrote:
> > > >> >> >>> Hi devs,
> > > >> >> >>>
> > > >> >> >>> Running mvn dependency:dependency-analyze produces
> interesting
> > > >> results.
> > > >> >> >>>
> > > >> >> >>> For example:
> > > >> >> >>>
> > > >> >> >>> [INFO]
> > > >> >>
> > > >>
> > >
> >
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> > > >> >> >>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
> > > >> >> >>> [INFO]
> > > >> >>
> > > >>
> > >
> >
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> > > >> >> >>> …
> > > >> >> >>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli)
> @
> > > >> >> xwiki-commons-properties ---
> > > >> >> >>> [WARNING] Used undeclared dependencies found:
> > > >> >> >>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
> > > >> >> >>> [WARNING]    javax.inject:javax.inject:jar:1:compile
> > > >> >> >>> [WARNING] Unused declared dependencies found:
> > > >> >> >>> [WARNING]
> > > >> >>
> > >  org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
> > > >> >> >>> [WARNING]
> > > >>  org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
> > > >> >> >>> [WARNING]
> > >  org.hibernate:hibernate-validator:jar:4.2.0.Final:test
> > > >> >> >>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
> > > >> >> >>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
> > > >> >> >>>
> > > >> >> >>> The question is (for this module but more generally for all
> > > others):
> > > >> >> >>> * Should we add slf4j and javax.inject reps in the pom.xml
> for
> > > this
> > > >> >> module? (for ex today slf4j and javax.inject are found in the
> > > >> component-api
> > > >> >> dep)
> > > >> >> >>>
> > > >> >> >>> I think we should, wdyt?
> > > >> >> >>
> > > >> >> >> +1 as well.
> > > >> >> >
> > > >> >> > hmm actually we need to decide about the following:
> > > >> >> >
> > > >> >> > * In order to simplify writing pom.xml for modules having
> > > components
> > > >> >> (i.e. depending on xwiki-commons-component-api) I had added the
> > > >> following
> > > >> >> to  xwiki-commons-component-api/pom.xml:
> > > >> >> >
> > > >> >> >    <!-- Make it easy for components that wish to log - They
> don't
> > > have
> > > >> >> to explicitly import SLF4J -->
> > > >> >> >    <dependency>
> > > >> >> >      <groupId>org.slf4j</groupId>
> > > >> >> >      <artifactId>slf4j-api</artifactId>
> > > >> >> >    </dependency>
> > > >> >> >
> > > >> >> > * In the same manner we have a dependency on javax.inject in
> > > >> >> xwiki-commons-component-api/pom.xml:
> > > >> >> >
> > > >> >> >    <!-- We add this dependency here so that users of the
> > Component
> > > API
> > > >> >> just need to depend on this artifact and
> > > >> >> >         don't have to explicitly add a dependency on
> > > >> >> javax.inject:java.inject. -->
> > > >> >> >    <dependency>
> > > >> >> >      <groupId>javax.inject</groupId>
> > > >> >> >      <artifactId>javax.inject</artifactId>
> > > >> >> >      <version>1</version>
> > > >> >> >    </dependency>
> > > >> >> >
> > > >> >> > So the question is: do we want to force each module depending
> on
> > > >> >> xwiki-commons-component-api to have to declare an explicit dep on
> > > >> >> javax.inject and org.slf4j?
> > > >> >> >
> > > >> >> > I'm not so sure about that…
> > > >> >>
> > > >> >> I'm -0 and near -1 to list this kind of dependencies, using slf4j
> > or
> > > >> >> javax.inject are what you HAVE TO use when you write an XWiki
> > > >> >> component so it's redundant from my POV.
> > > >> >>
> > > >> >> >
> > > >> >> > WDYT?
> > > >> >> >
> > > >> >> > Thanks
> > > >> >> > -Vincent
> > > >> >> >
> > > >> >> >>> Note that the "Unused declared dependencies found:" doesn't
> > > always
> > > >> >> generate correct results as is the case here. This is mostly
> > because
> > > >> it's a
> > > >> >> static byte code check so any dep used at runtime will be
> > considered
> > > >> unused.
> > > >> >> >>> See
> > > >> >>
> > > >>
> > >
> >
> http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
> > > >> >> >>
> > > >> >> >> Some of these dependencies are not used directly by us, but
> are
> > > >> needed
> > > >> >> >> transitively by another library. For example, slf4j needs
> > logback,
> > > >> which
> > > >> >> >> we never use directly (although we don't really declare it in
> > > every
> > > >> >> >> module that does logging). Hibernate needs us to pick a
> cache, a
> > > >> >> >> connection pool, validator, and a bytecode manipulation
> utility.
> > > So
> > > >> yes,
> > > >> >> >> we can safely ignore most of these false negatives, but we
> > should
> > > >> still
> > > >> >> >> try to remove those that are really wrongfully declared as
> > > >> dependencies.
> > > >> >> >>
> > > >> >> >>> Thanks
> > > >> >> >>> -Vincent
> > > >> >> >
> > > >> >> > _______________________________________________
> > > >> >> > devs mailing list
> > > >> >> > [hidden email]
> > > >> >> > http://lists.xwiki.org/mailman/listinfo/devs
> > > >> >> >
> > > >> >>
> > > >> >>
> > > >> >>
> > > >> >> --
> > > >> >> Thomas Mortagne
> > > >> >> _______________________________________________
> > > >> >> devs mailing list
> > > >> >> [hidden email]
> > > >> >> http://lists.xwiki.org/mailman/listinfo/devs
> > > >> >>
> > > >> >
> > > >> >
> > > >> >
> > > >> > --
> > > >> > Denis Gervalle
> > > >> > SOFTEC sa - CEO
> > > >> > _______________________________________________
> > > >> > devs mailing list
> > > >> > [hidden email]
> > > >> > http://lists.xwiki.org/mailman/listinfo/devs
> > > >>
> > > >>
> > > >>
> > > >> --
> > > >> Thomas Mortagne
> > > >> _______________________________________________
> > > >> devs mailing list
> > > >> [hidden email]
> > > >> http://lists.xwiki.org/mailman/listinfo/devs
> > > >>
> > > >
> > > >
> > > >
> > > > --
> > > > Denis Gervalle
> > > > SOFTEC sa - CEO
> > > > _______________________________________________
> > > > devs mailing list
> > > > [hidden email]
> > > > http://lists.xwiki.org/mailman/listinfo/devs
> > >
> > >
> > >
> > > --
> > > Thomas Mortagne
> > > _______________________________________________
> > > devs mailing list
> > > [hidden email]
> > > http://lists.xwiki.org/mailman/listinfo/devs
> > >
> >
> >
> >
> > --
> > Denis Gervalle
> > SOFTEC sa - CEO
> > _______________________________________________
> > devs mailing list
> > [hidden email]
> > http://lists.xwiki.org/mailman/listinfo/devs
> >
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs
>



--
Denis Gervalle
SOFTEC sa - CEO
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Cleaning our POM deps by explicitly listing used dependencies?

jerem
Le 5 juin 2014 19:44, "Denis Gervalle" <[hidden email]> a écrit :
>
> On Thu, Jun 5, 2014 at 3:51 PM, Jeremie BOUSQUET <
[hidden email]

> > wrote:
>
> > Hi,
> >
> >
> > 2014-06-05 15:10 GMT+02:00 Denis Gervalle <[hidden email]>:
> >
> > > On Thu, Jun 5, 2014 at 2:27 PM, Thomas Mortagne <
> > [hidden email]
> > > >
> > > wrote:
> > >
> > > > On Thu, Jun 5, 2014 at 2:14 PM, Denis Gervalle <[hidden email]>
wrote:
> > > > > What happen if you also use dependency A not just because of B ?
> > > >
> > > > You put a dependency on A.
> > > >
> > >
> > > But you may not see that so easily when you change a few line in an
> > > existing module. Nothing will complains until you remove your deps to
B.

> > >
> > >
> > > >
> > > > > How do you determine "because of B" ?
> > > >
> > > > By thinking.
> > > >
> > >
> > > Ok, I rephrase my question.
> > > Could you define what you consider a usage of dependency A because of
B
> > and
> > > the opposite ?
> > >
> > >
> > If I understood, you depend on B, and by transitivity, you also depend
on A
> > (indirectly).
> > Then, from your module you use some classes of A (brought to you
> > indirectly, so "magically", it compiles).
> > So you never explicitly declare the dep on A from your module. If one
day
> > you remove dependency on B for any reason, you will also loose A (and
also
> > you can say it's an explicit dependency that is not "explicit", though
it's
> > a matter of taste or point of view to consider transitive deps not to be
> > explicit ...).
> >
> > The opposite would be to declare explicit dep on A from your module,
while
> > you don't need it at all - only B needs it.
> >
>
> You get it fully. The question is, could we define in a few word when the
> transitivity could be used, because the usage of A is just "because of" B
?

> Else, we should add a deps on A as soon as we import any class from A IMO.
>
>
> >
> >
> >
> > >
> > > >
> > > > >
> > > > > And what to you think of xwiki-commons-test-component that is a
deps

> > of
> > > > > xwiki-platform-core ?
> > > >
> > > > It's wrong IMO. Any forced dependency is wrong IMO.
> > > >
> > > > > Should we remove it ?
> > > >
> > > > Yes we should remove it.
> > > >
> > >
> > > Why do we get it ? Its removal could become an nightmare... but if we
> > agree
> > > on that, we should remove it ASAP.
> > >
> > >
> > > >
> > > > > What about deps for logging ?
> > > >
> > > > Depends how you use it, the logger used with @Inject is an official
> > > > feature of our component framework so xwiki-commons-component-api
> > > > should be enough.
> > > >
> > > > > And could we add xwiki-commons-stability (probably provided scope)
> > to a
> > > > > high level pom to avoid adding/removing it all the time ? (or
forget
> > > it,
> > > > > since it come with xwiki-commons-component-api currently) ?
> > > >
> > > > It's far from being used everywhere and there is no rule forcing to
> > > > use it, you set @Unstable when an API is unstable, it's not
forbidden
> > > > to not go through @Unstable. Plus you are supposed to remove that
> > > > annotation after some time.
> > > >
> > >
> > > Ok, so not deps at any scope in any high level poms. This seems
opposite
> > to
> > > what Sergiu proposed, but it would be nice to agree on a rule.
> > >
> > > To sum up, currently I am not sure the exception rule "because of" is
> > clear
> > > enough to not create confusion. I also agree with Sergiu that we
should
> > > list all (no warning of used deps not declared in
dependency:analysis),

> > > this make the rule clear at least. I am not against factoring common
> > > infrastructure in a single place, but Thomas seems to be clearly -1.
> > >
> >
> > I'm not sure it relates to what you describe above. Factoring deps in a
> > common infrastructure is more a matter of using dependencyManagement at
> > correct level (top-most usually), but the rule to declare only deps that
> > are really needed per module is a very widely used best practice.
> >
> > From a contributor (not committer) point of view, it's sometimes
annoying
> > to have to update your builds because of this kind of changes.
Obviously it
> > would be difficult to consider those dependency graphs as APIs :) , but
at
> > the same time when they evolve too frequently, it can be considered
painful
> > to follow by devs in general ...
> > Some are not impacting, but for example regarding @Unstable, if it's not
> > brought anymore by xwiki-commons-component-api, it will break my build
when

> > I upgrade xwiki versions.
> >
>
> This is bad, since I do not see any reason for xwiki-commons-component-api
> to bring xwiki-commons-stability to you.
>
>
> >
> > With Maven, it's not really a "bad" practice, to consider that some poms
> > are used mainly to bring to you a set of logically related dependencies
by
> > transitivity (there's even the "import" scope, though it's not the best
> > sample of good practice I agree). Question is more (case by case), is it
> > really "bad" to bring @Unstable by default to everyone that develops an
> > xwiki component, even if he will never use it / don't use it anymore ?
> >
>
> I would be positive, especially because it is not a runtime deps, and we
> can simply use the "provided" scope.
> Thomas do not seems to agree.

I agree with Thomas, forced dependencies in your build are bad. The only
forced dep I ever used in my root poms is junit, but here the objective was
really to force it ;)
In this case above, it would not be a pom really part of your build, but a
pom to be used as a dependency by devs users of xwiki apis. That's why I
talked about "import" scope, it's the typical use-case for it.
But I agree that in that case it's more maintenance work, additional poms
and deps graphs, so maybe not worth it at all.
I'm not sure I'd do it if I were you, even if I 'd find it nice as
contributor ;)

>
> All I really want at the end, is that we have clear rule.
> Unless we can define what "because of" is, the only rule I see valid
> currently, would be to explicitly depends on any module we do an import
> from.
>
>
> >
> > >
> > >
> > > It would be nice to have more feedback from other committers ! This is
> > not
> > > a minor aspect of our best practice IMO.
> > >
> > >
> > > >
> > > > >
> > > > > WDYT ?
> > > > >
> > > > >
> > > > >
> > > > > On Thu, Jun 5, 2014 at 11:42 AM, Thomas Mortagne <
> > > > [hidden email]>
> > > > > wrote:
> > > > >
> > > > >> For me the rule to apply is simple: when you require dependency A
> > > > >> because of another dependency B (B expose A in it's API, you
> > implement
> > > > >> an interface of A to be found by B, etc.) you should not
explicitly
> > > > >> depend on A.
> > > > >>
> > > > >> On Thu, Jun 5, 2014 at 11:32 AM, Denis Gervalle <[hidden email]>
> > > wrote:
> > > > >> > Hi devs,
> > > > >> >
> > > > >> > I am reviving this proposal since we never came to a
conclusion,
> > > and I
> > > > >> have
> > > > >> > the feeling that our deps become more and more convoluted.
> > > > >> >
> > > > >> > To resume what I get from past history with my own vision:
> > > > >> >
> > > > >> > 1) Since our modules are getting really modular, it should
never
> > > > silently
> > > > >> > depends on transitive dependency of another module (with IMO
some
> > > > >> > exception, see 3). So any undeclared deps found by
> > > dependency:analyse
> > > > >> > should be explicitly declare in the effective pom (Vincent POV
as

> > > > well)
> > > > >> > 2) Apply maven principle, we should reuse and apply
> > > > >> > convention-over-configuration
> > > > >> > over configuration, so common dependency like slf4j,
> > > > >> xwiki-commons-stability
> > > > >> > ? ... should be in a high level parent pom
> > > > >> > 3) We may rely on some very tight transitive dependency, for
> > > exemple,
> > > > it
> > > > >> > would be really curious that xwiki-commons-component-api stop
> > > > providing
> > > > >> > javax.inject, or that xwki-commons-test-components stop
providing
> > > > junit,
> > > > >> > mockito, and al.
> > > > >> >
> > > > >> > It would be nice to add those rules in our best practice and to
> > > always
> > > > >> > check our pom upon finishing changes in a module. The best
would
> > be
> > > to
> > > > >> > enforce those rules, but this is not easy since static
analysis is

> > > > >> limited
> > > > >> > and could create false positive.
> > > > >> >
> > > > >> > WDYT ?
> > > > >> >
> > > > >> >
> > > > >> > On Mon, Aug 15, 2011 at 10:31 PM, Thomas Mortagne <
> > > > >> [hidden email]
> > > > >> >> wrote:
> > > > >> >
> > > > >> >> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <
> > > [hidden email]>
> > > > >> >> wrote:
> > > > >> >> >
> > > > >> >> > On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote:
> > > > >> >> >
> > > > >> >> >> On 08/12/2011 07:50 AM, Vincent Massol wrote:
> > > > >> >> >>> Hi devs,
> > > > >> >> >>>
> > > > >> >> >>> Running mvn dependency:dependency-analyze produces
> > interesting
> > > > >> results.
> > > > >> >> >>>
> > > > >> >> >>> For example:
> > > > >> >> >>>
> > > > >> >> >>> [INFO]
> > > > >> >>
> > > > >>
> > > >
> > >
> >
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> > > > >> >> >>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT
> > > > >> >> >>> [INFO]
> > > > >> >>
> > > > >>
> > > >
> > >
> >
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> > > > >> >> >>> …
> > > > >> >> >>> [INFO] --- maven-dependency-plugin:2.3:analyze
(default-cli)
> > @
> > > > >> >> xwiki-commons-properties ---
> > > > >> >> >>> [WARNING] Used undeclared dependencies found:
> > > > >> >> >>> [WARNING]    org.slf4j:slf4j-api:jar:1.6.1:compile
> > > > >> >> >>> [WARNING]    javax.inject:javax.inject:jar:1:compile
> > > > >> >> >>> [WARNING] Unused declared dependencies found:
> > > > >> >> >>> [WARNING]
> > > > >> >>
> > > >
 org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile
> > > > >> >> >>> [WARNING]
> > > > >>  org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test
> > > > >> >> >>> [WARNING]
> > > >  org.hibernate:hibernate-validator:jar:4.2.0.Final:test
> > > > >> >> >>> [WARNING]    org.hamcrest:hamcrest-core:jar:1.1:test
> > > > >> >> >>> [WARNING]    org.jmock:jmock:jar:2.5.1:test
> > > > >> >> >>>
> > > > >> >> >>> The question is (for this module but more generally for
all

> > > > others):
> > > > >> >> >>> * Should we add slf4j and javax.inject reps in the pom.xml
> > for
> > > > this
> > > > >> >> module? (for ex today slf4j and javax.inject are found in the
> > > > >> component-api
> > > > >> >> dep)
> > > > >> >> >>>
> > > > >> >> >>> I think we should, wdyt?
> > > > >> >> >>
> > > > >> >> >> +1 as well.
> > > > >> >> >
> > > > >> >> > hmm actually we need to decide about the following:
> > > > >> >> >
> > > > >> >> > * In order to simplify writing pom.xml for modules having
> > > > components
> > > > >> >> (i.e. depending on xwiki-commons-component-api) I had added
the

> > > > >> following
> > > > >> >> to  xwiki-commons-component-api/pom.xml:
> > > > >> >> >
> > > > >> >> >    <!-- Make it easy for components that wish to log - They
> > don't
> > > > have
> > > > >> >> to explicitly import SLF4J -->
> > > > >> >> >    <dependency>
> > > > >> >> >      <groupId>org.slf4j</groupId>
> > > > >> >> >      <artifactId>slf4j-api</artifactId>
> > > > >> >> >    </dependency>
> > > > >> >> >
> > > > >> >> > * In the same manner we have a dependency on javax.inject in
> > > > >> >> xwiki-commons-component-api/pom.xml:
> > > > >> >> >
> > > > >> >> >    <!-- We add this dependency here so that users of the
> > > Component
> > > > API
> > > > >> >> just need to depend on this artifact and
> > > > >> >> >         don't have to explicitly add a dependency on
> > > > >> >> javax.inject:java.inject. -->
> > > > >> >> >    <dependency>
> > > > >> >> >      <groupId>javax.inject</groupId>
> > > > >> >> >      <artifactId>javax.inject</artifactId>
> > > > >> >> >      <version>1</version>
> > > > >> >> >    </dependency>
> > > > >> >> >
> > > > >> >> > So the question is: do we want to force each module
depending
> > on
> > > > >> >> xwiki-commons-component-api to have to declare an explicit
dep on
> > > > >> >> javax.inject and org.slf4j?
> > > > >> >> >
> > > > >> >> > I'm not so sure about that…
> > > > >> >>
> > > > >> >> I'm -0 and near -1 to list this kind of dependencies, using
slf4j

> > > or
> > > > >> >> javax.inject are what you HAVE TO use when you write an XWiki
> > > > >> >> component so it's redundant from my POV.
> > > > >> >>
> > > > >> >> >
> > > > >> >> > WDYT?
> > > > >> >> >
> > > > >> >> > Thanks
> > > > >> >> > -Vincent
> > > > >> >> >
> > > > >> >> >>> Note that the "Unused declared dependencies found:"
doesn't

> > > > always
> > > > >> >> generate correct results as is the case here. This is mostly
> > > because
> > > > >> it's a
> > > > >> >> static byte code check so any dep used at runtime will be
> > > considered
> > > > >> unused.
> > > > >> >> >>> See
> > > > >> >>
> > > > >>
> > > >
> > >
> >
http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html
> > > > >> >> >>
> > > > >> >> >> Some of these dependencies are not used directly by us, but
> > are
> > > > >> needed
> > > > >> >> >> transitively by another library. For example, slf4j needs
> > > logback,
> > > > >> which
> > > > >> >> >> we never use directly (although we don't really declare it
in

> > > > every
> > > > >> >> >> module that does logging). Hibernate needs us to pick a
> > cache, a
> > > > >> >> >> connection pool, validator, and a bytecode manipulation
> > utility.
> > > > So
> > > > >> yes,
> > > > >> >> >> we can safely ignore most of these false negatives, but we
> > > should
> > > > >> still
> > > > >> >> >> try to remove those that are really wrongfully declared as
> > > > >> dependencies.
> > > > >> >> >>
> > > > >> >> >>> Thanks
> > > > >> >> >>> -Vincent
> > > > >> >> >
> > > > >> >> > _______________________________________________
> > > > >> >> > devs mailing list
> > > > >> >> > [hidden email]
> > > > >> >> > http://lists.xwiki.org/mailman/listinfo/devs
> > > > >> >> >
> > > > >> >>
> > > > >> >>
> > > > >> >>
> > > > >> >> --
> > > > >> >> Thomas Mortagne
> > > > >> >> _______________________________________________
> > > > >> >> devs mailing list
> > > > >> >> [hidden email]
> > > > >> >> http://lists.xwiki.org/mailman/listinfo/devs
> > > > >> >>
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> > --
> > > > >> > Denis Gervalle
> > > > >> > SOFTEC sa - CEO
> > > > >> > _______________________________________________
> > > > >> > devs mailing list
> > > > >> > [hidden email]
> > > > >> > http://lists.xwiki.org/mailman/listinfo/devs
> > > > >>
> > > > >>
> > > > >>
> > > > >> --
> > > > >> Thomas Mortagne
> > > > >> _______________________________________________
> > > > >> devs mailing list
> > > > >> [hidden email]
> > > > >> http://lists.xwiki.org/mailman/listinfo/devs
> > > > >>
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Denis Gervalle
> > > > > SOFTEC sa - CEO
> > > > > _______________________________________________
> > > > > devs mailing list
> > > > > [hidden email]
> > > > > http://lists.xwiki.org/mailman/listinfo/devs
> > > >
> > > >
> > > >
> > > > --
> > > > Thomas Mortagne
> > > > _______________________________________________
> > > > devs mailing list
> > > > [hidden email]
> > > > http://lists.xwiki.org/mailman/listinfo/devs
> > > >
> > >
> > >
> > >
> > > --
> > > Denis Gervalle
> > > SOFTEC sa - CEO
> > > _______________________________________________
> > > devs mailing list
> > > [hidden email]
> > > http://lists.xwiki.org/mailman/listinfo/devs
> > >
> > _______________________________________________
> > devs mailing list
> > [hidden email]
> > http://lists.xwiki.org/mailman/listinfo/devs
> >
>
>
>
> --
> Denis Gervalle
> SOFTEC sa - CEO
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs