SEC-1541: CLONE -all modules depend directly on commons logging #1783

Closed
spring-issuemaster opened this Issue Aug 16, 2010 · 10 comments

1 participant

@spring-issuemaster

jieryn (Migrated from SEC-1541) said:

Please remove commons logging dependency from all modules except of core. All modules will still be commons logging dependent and exclusion of commons logging in projects using slf4j will be simpler.

Spring framework uses same system.

@spring-issuemaster

jieryn said:

Please do NOT remove commons logging dependency. This is wrong. If the module requires commons-logging to compile, which it does, and especially if it directly does, which is also true, then it should be listed as a dependency. It is wrong and poor form to rely on a transitive dependency to provide it since the transitive dependency may remove that dependency themselves. Then this package would break as consequence.

Yes, I realize that the original issue opener is lazy (which is good), but this solution is just plain bad.

There's a far easier way of accomplishing the task for the lazy among us, and that is to simply put a dependency on commons-logging and mark it as provided. This will prevent Maven from pulling in the dependency in the traditional sense and instead mark it as already being available from some outside means--the means will be you, and your placing a dependency on some other implementation like jcl-over-slf4j.

QED

@spring-issuemaster

Luke Taylor said:

Is this actually something that is causing you a problem?

The dependency in this case is via spring-core which we already know the transitive dependencies of prior to each Spring Security release. Adding a 'provided' dependency on commons-logging is therefore pretty pointless as provided deps are only relevant for building the project itself and anyone who is using Spring Security must also have spring-core in their build. In theory I agree about the use of transitive dependencies in a build. It is good to know what your project actually needs to compile. However, maven makes it impossible to differentiate between direct dependencies and those which are transitive, so it is actually pretty hard to work out what the compile-time dependencies of your project are when using maven - it just doesn't provide that information and the poms are not a good record of it.

@spring-issuemaster

jieryn said:

I am not suggesting that Spring, Spring Security, nor any derivative project at SpringSource, put a dependency on commons-logging. I was suggesting that the original JIRA requester to remove commons-logging dependency should do so [example below]. We have a compile time dependency on commons-logging, not listing it is just wrong.

To the lazy user:

<dependencies>
  <!-- set commons-logging to be already provided so that we do not have to
   laboriously exclude it from every package under the sun -->
  <dependency>
    <groupId>commons-logging</groupId>
    <artifactId>commons-logging</artifactId>
    <scope>provided</scope>
  </dependency>
  <dependency>
    <groupId>org.slf4j</groupId>
    <artifactId>slf4j-api</artifactId>
  </dependency>
  <dependency>
    <groupId>org.slf4j</groupId>
    <artifactId>jcl-over-slf4j</artifactId>
  </dependency>
  <!-- rest of my normal dependencies -->
</dependencies>

Done. No longer worry about Spring having dependencies on commons-logging. No longer have to specify exclusions for every single dependency on Spring or any module Spring produces. But for Spring products themselves to not correctly specify their compile time requirements is a bug. Yes, it affects me -- we have some in-house Maven plugins which produce dependency tree graphs to do analysis and one report lists missing dependencies (glorified/prettified dependency:analyze).

@spring-issuemaster

Luke Taylor said:

If you are using maven poms to work out the actual compile-time dependencies of other projects, then I would imagine commons-logging would be the least of your worries. I doubt if many projects in maven central have "compile" dependencies which exactly match what would need to be added to the javac classpath to compile the code (assuming that's your criteria). I don't think it's a bug - it's just an inherent problem with the way maven treats dependencies. I'm curious as to how you would suggest to a maven user that they should actually establish the compile-time dependencies which they should add to the pom and also how they would maintain that list going forward.

@spring-issuemaster

jieryn said:

I think you're missing the point so I'll reiterate it step by step:

1) Spring Security has an actual compile time dependency on commons-logging
2) Spring Security 3.0.2 properly listed this compile time dependency in the POM
3a) Spring Security 3.0.3 has removed the compile time dependency listing in the POM
3b) Spring Security 3.0.3 has not removed the actual compile time requirement

What I choose to do with the information seems irrelevant. Spring Security is not properly describing its requirements to the build infrastructure. This is not a Maven bug, it is a Spring Security bug.

** Spring Security has a compile time dependency and is not listing it **

I'm really struggling here because it's so patently obvious that this is an issue and has nothing to do with whether other artifacts in Maven central properly list their dependencies. When I find such projects, I open bugs in their issue trackers too. This has nothing to do with Maven resolution of dependencies, transitive or otherwise. If anything, I would be more easily convinced that it is a Maven bug to allow a project like Spring Security to compile when it doesn't list all its required dependencies and instead relies on transitive dependencies to satisfy its requirements.

Your question about what I would suggest to users trying to deal with broken packages, like Spring Security, that do not properly list their compile time dependencies is a bit strange; I'm not sure I understand it fully but I'll take a whack at it.. Open JIRAs in the project issue tracker and get the developers to fix their broken systems, as I am doing now.

@spring-issuemaster

Luke Taylor said:

Ok, leaving aside commons-logging for a moment, let me rephrase my question in two parts.

  1. What is your definition of a "compile" dependency?
  2. How will you know what the "compile" dependencies of those projects are in order to raise those jira issues? In fact, how do you know what the "compile" dependencies of your own projects are if you are using maven?

Maven always adds transitive dependencies to the compile classpath, so you have no way of working out what you actually require to compile. Since that's the case with maven in general, I'm not really bothered about commons-logging. As I mentioned in the original issue, we will not be using maven for future releases. In the current gradle build, we compile against commons-logging, but run the tests against slf4j/logback. We also have transitive dependency resolution disabled for the compile task, so the the listed dependencies actually are those which would be required to run javac against the code.

@spring-issuemaster

jieryn said:

Q1. What is your definition of a "compile" dependency?
A1. Some module other than the current one which is required for this module to be compiled. Most of the time this will be because you issued an import org.apache.commons.logging.Log; statement, as a topical example.

Q2. How will you know what the "compile" dependencies of those projects are in order to raise those jira issues? In fact, how do you know what the "compile" dependencies of your own projects are if you are using maven?
A2. I don't see how this is relevant, but ever the teacher, there are lots of easy ways to determine this. It's quite trivial to ensure that every import can be found in one of the dependencies which are immediately referenced in the POM. Maven provides a nice plugin which assists in this process, see http://maven.apache.org/plugins/maven-dependency-plugin/analyze-mojo.html

Maven does not always add transitive dependencies to the compile classpath. For example, junit is a frequent dependency of a most projects, and yet it will not be added to the compile classpath. It will be added to the test-compile classpath. I could go on and on..

Use whatever deployment mechanism you want, but as long as you are going to make POMs available to the general community, then you should get them correct. I think it's a shame that your team would abandon Maven, but perhaps it's based on obvious misunderstandings and misapprehensions about how Maven works, as is evident in this JIRA; but again, use whatever deployment mechanism you wish.. Please don't introduce problems for majority of us.

So I recommend we either delete the POMs all together, or get them right. Wrong/bad information is worse than no information.

@spring-issuemaster

jieryn said:

I forgot to mention,

$ git clone git://git.springsource.org/spring-security/spring-security.git
$ fgrep -r 'import org.apache.commons.logging.Log' spring-security | wc
    192     384   27160
$ echo "Obviously there is a compile-time dependency on commons-logging."
@spring-issuemaster

Luke Taylor said:

Ok, so your definition is basically the one I already mentioned - javac's requirements. This may also include dependencies which contain classes which you do not use anywhere in your code (because of inheritance issues, for example). So from my perspective, the simplest way of working out whether your compile-time dependencies are adequate is to run the compiler against them. That's what we do now, and you can't do it with maven.

The existing build poms will be deleted, and the deployed ones will be generated individually for each module, functioning purely as dependency metadata.

@spring-issuemaster

jieryn said:

I think you're unable to fully grasp the words that I'm putting down here, let alone an advanced argument for correctness. The POMs should be deleted, and I'd like to see the JIRA or revision which does this please..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment