Bug 43304 - [PATCH] Maven POM dependencies
[PATCH] Maven POM dependencies
Status: RESOLVED FIXED
Product: Log4j
Classification: Unclassified
Component: Other
1.2
All All
: P2 regression
: ---
Assigned To: log4j-dev
http://repo1.maven.org/maven2/log4j/l...
:
: 43452 43488 48216 48991 (view as bug list)
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2007-09-04 04:55 UTC by Jo Vandermeeren
Modified: 2010-03-26 05:39 UTC (History)
7 users (show)



Attachments
Tweaks dependencies to add optional flag (1013 bytes, patch)
2007-09-13 20:10 UTC, Paul Smith
Details | Diff
Newer version with oro not marked as optional (oro is test scope already) (833 bytes, patch)
2007-09-13 20:12 UTC, Paul Smith
Details | Diff
This patch reformats the xml by changing whitespace (26.45 KB, patch)
2007-11-27 14:23 UTC, Dennis Lundberg
Details | Diff
Apart from whitespace this patch also adds versions for all plugins (27.92 KB, patch)
2008-03-16 12:17 UTC, Dennis Lundberg
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jo Vandermeeren 2007-09-04 04:55:33 UTC
Log4j release 1.2.15 enforces some runtime dependencies (javax.jms, etc) which
should have been marked as optional in the Maven POM, but they are not.
Comment 1 Curt Arnold 2007-09-04 21:22:09 UTC
Background info: http://maven.apache.org/guides/introduction/introduction-to-optional-and-
excludes-dependencies.html

Neither optional dependencies or exclude dependencies (which could be used as a work-around with 
1.2.15) are desirable.  But log4j 1.2.x isn't a candidate to be split into subprojects with only non-
optional dependencies.

The log4j 1.2.14 POM (and likely proceeding ones) were not actually used to build log4j and listed no 
dependencies (or effectively everything was an optional dependency).

I could see how it would be desirable to make the compile scope dependencies (javamail, jms, jmxri, 
jmxtools) optional in 1.2.16.  Or maybe all but javamail since it is in java.net repo and is more likely to 
be used than the others which need to be manually downloaded.
Comment 2 Paul Smith 2007-09-04 21:27:25 UTC
the problem here is that using log4j 1.2.15 in a project will immediately
require that upstream project to depend on java mail and jms etc, when in
reality, this is not the case _unless_ you need to use certain features.

Ideally the upstream projects when depending on log4j only need log4j itself. 
If that project then decides to need to use, say, the SMTPAppender, that
upstream project will need to mark itself as depending on javax.mail.

I'm in favour of keeping log4j's mandatory dependencies as small as possible (it
should be zero).
Comment 3 Paul Smith 2007-09-13 20:10:59 UTC
Created attachment 20824 [details]
Tweaks dependencies to add optional flag

Attached patch which changes log4j's dependencies to be marked as optional.  I
borrowed this from how the ActiveMQ team have specified it.

Taken from the POM reference guide @ maven.apache.org:

"In the shortest terms, optional  lets other projects know that, when you use
this project, you do not require this dependency in order to work correctly."
Comment 4 Paul Smith 2007-09-13 20:12:33 UTC
Created attachment 20825 [details]
Newer version with oro not marked as optional (oro is test scope already)

Just realised ORO is only dependency because of test scope, so that one doesn't
need to be marked as optional.
Comment 5 Curt Arnold 2007-09-14 15:01:48 UTC
Committed rev 575804.
Comment 6 Jacob Kjome 2007-09-23 20:27:31 UTC
*** Bug 43452 has been marked as a duplicate of this bug. ***
Comment 7 Dennis Kieselhorst 2007-09-26 23:34:05 UTC
*** Bug 43488 has been marked as a duplicate of this bug. ***
Comment 8 Gerd Aschbrenner 2007-10-29 06:35:44 UTC
Environmental dependencies like 'javax.mail' or 'javax.serlvet-api' should be
set to scope 'provided'.

Example to explain what I mean:
When a feature uses the dependency 'javax.ejb-2.1' it usually will only be used
in an environment still providing this dependency (J2EE-Server). The scope of
such dependencies should be set to 'provided'.

I guess at least the dependencies 'javax.mail' and 'javax.jms' should get the
scope 'provided'. I do not know enough about the jmx-libs, but I guess they also
can be set to 'provided'.


The 'optional' tag may stay. When they stay, a comment would be nice listing the
optional features that are in need of it. That would help others to decide to
include or exclude this dependency.
Something like: Needed when the MailAppender is used.


Comment 9 Dennis Lundberg 2007-11-27 14:04:03 UTC
The javax.mail dependency should not be marked as provided. Provided means that
the dependency will always be provided by the target environment.

An example to highlight why it would be wrong to mark it as provided. Say you
have a command line application that uses log4j for logging. The user of the
application decides to add an SMTPAppender to send email if something goes
fataly wrong. Now the user needs to take action to make this work. He or she
needs to copy the javamail jar into the classpath of the application. In other
words: javax.mail is not provided by the target environment.
Comment 10 Dennis Lundberg 2007-11-27 14:23:20 UTC
Created attachment 21196 [details]
This patch reformats the xml by changing whitespace

This patch reformats the xml in the pom with a consistent 2-spaces indentation.
Only whitespace is changed in this patch.

Another patch with improvements to the actual content is coming soon.
Comment 11 Dennis Lundberg 2008-03-16 12:17:35 UTC
Created attachment 21674 [details]
Apart from whitespace this patch also adds versions for all plugins

This includes the whitespace changes in my previous patch.

The patch adds versions for all plugins (it's a best practice to do that) and moves the release-plugin from reporting to build.
Comment 12 Paul Smith 2008-05-12 15:34:28 UTC
Thanks Dennis, I've applied your patch to the pom.  Looks puuurty now!
Comment 13 Dennis Lundberg 2010-02-11 19:11:33 UTC
*** Bug 48216 has been marked as a duplicate of this bug. ***
Comment 14 Curt Arnold 2010-03-26 05:39:32 UTC
*** Bug 48991 has been marked as a duplicate of this bug. ***