5 Jun
2014
5 Jun
'14
7:43 p.m.
On Thu, Jun 5, 2014 at 3:51 PM, Jeremie BOUSQUET <jeremie.bousquet(a)gmail.com
Why do we get it ? Its removal could become an nightmare... but if we agree
Ok, so not deps at any scope in any high level poms. This seems opposite to
This is bad, since I do not see any reason for xwiki-commons-component-api
to bring xwiki-commons-stability to you.
wrote:
> Hi,
>
>
> 2014-06-05 15:10 GMT+02:00 Denis Gervalle <dgl(a)softec.lu>lu>:
>
> > On Thu, Jun 5, 2014 at 2:27 PM, Thomas Mortagne <
> thomas.mortagne(a)xwiki.com
> > >
> wrote:
> >
> > > On Thu, Jun 5, 2014 at 2:14 PM, Denis Gervalle <dgl(a)softec.lu 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.
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.
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 ?
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 <
> > > thomas.mortagne(a)xwiki.com>
> > > 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
<dgl(a)softec.lu>
> 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 <
> > > >> thomas.mortagne(a)xwiki.com
> > > >> > wrote:
> > >
>> >
> > > >> >> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <
> > vincent(a)massol.net>
> > > >> > 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-dependen…
> > > >> >> >>
> > > >> >> >> 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
> > > >> >> > devs(a)xwiki.org
> > > >> >> > http://lists.xwiki.org/mailman/listinfo/devs
> > > >> >> >
> > > >> >>
> > > >> >>
> > > >> >>
> > > >> >> --
> > > >> >> Thomas Mortagne
> > > >> >> _______________________________________________
> > > >> >> devs mailing list
> > > >> >> devs(a)xwiki.org
> > > >> >> http://lists.xwiki.org/mailman/listinfo/devs
> > > >> >>
> > > >> >
> > > >> >
> > > >> >
> > > >> > --
> > > >> > Denis Gervalle
> > > >> > SOFTEC sa - CEO
> > > >> > _______________________________________________
> > > >> > devs mailing list
> > > >> > devs(a)xwiki.org
> > > >> > http://lists.xwiki.org/mailman/listinfo/devs
> > > >>
> > > >>
> > > >>
> > > >> --
> > > >> Thomas Mortagne
> > > >> _______________________________________________
> > > >> devs mailing list
> > > >> devs(a)xwiki.org
> > > >> http://lists.xwiki.org/mailman/listinfo/devs
> > > >>
> > > >
> > > >
> > > >
> > > > --
> > > > Denis Gervalle
> > > > SOFTEC sa - CEO
> > > > _______________________________________________
> > > > devs mailing list
> > > > devs(a)xwiki.org
> > > > http://lists.xwiki.org/mailman/listinfo/devs
> > >
> > >
> > >
> > > --
> > > Thomas Mortagne
> > > _______________________________________________
> > > devs mailing list
> > > devs(a)xwiki.org
> > > http://lists.xwiki.org/mailman/listinfo/devs
> > >
> >
> >
> >
> > --
> > Denis Gervalle
> > SOFTEC sa - CEO
> > _______________________________________________
> > devs mailing list
> > devs(a)xwiki.org
> > http://lists.xwiki.org/mailman/listinfo/devs
> >
> _______________________________________________
> devs mailing list
> devs(a)xwiki.org
> http://lists.xwiki.org/mailman/listinfo/devs
>
--
Denis Gervalle
SOFTEC sa - CEO