Maven Enforcer Plugin
  1. Maven Enforcer Plugin
  2. MENFORCER-128

Fail the build if a dependency is overwriten with an incompatible lower version (patch)

    Details

    • Type: New Feature New Feature
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.1
    • Component/s: Standard Rules
    • Labels:
      None
    • Testcase included:
      yes
    • Patch Submitted:
      Yes
    • Number of attachments :
      1

      Description

      Overwriting a dependency to a lower version than any of your other dependencies need should fail the build if this new enforcer rule is active.

      For example, this is bad:

      
        <dependencies>
          <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-api</artifactId>
            <version>1.4.0</version>
          </dependency>
          <dependency>
            <groupId>ch.qos.logback</groupId>
            <artifactId>logback-classic</artifactId>
            <version>0.9.9</version>
            <!-- Depends on org.slf4j:slf4j-api:1.5.0 -->
          </dependency>
        </dependencies>
      

      Attaching patch in a few minutes.

        Activity

        Hide
        Geoffrey De Smet added a comment -

        Patch attached.
        Please apply on this codebase:
        http://svn.apache.org/repos/asf/maven/enforcer/trunk/

        Show
        Geoffrey De Smet added a comment - Patch attached. Please apply on this codebase: http://svn.apache.org/repos/asf/maven/enforcer/trunk/
        Hide
        Geoffrey De Smet added a comment -

        Patch includes documentation and IT tests.

        Show
        Geoffrey De Smet added a comment - Patch includes documentation and IT tests.
        Hide
        Geoffrey De Smet added a comment -

        To see what sort of dirt this can bring to the surface in a big project, see this issue:
        https://issues.jboss.org/browse/JBRULES-3382

        Show
        Geoffrey De Smet added a comment - To see what sort of dirt this can bring to the surface in a big project, see this issue: https://issues.jboss.org/browse/JBRULES-3382
        Hide
        Paul Gier added a comment -

        Patch applied in r1242799, thanks!

        Show
        Paul Gier added a comment - Patch applied in r1242799 , thanks!
        Hide
        Robert Scholte added a comment -

        This new rule is called IncompatibleDependencyOverwrite, but I don't think that reflects the real purpose. A lower version can still be compatible. This rule is actually checking if it is using the highest of all defined versions per dependency. IMO something like ForceHighestDependencyVersion or ForceUpperBoundDependency would be a better name. WDYT?

        Show
        Robert Scholte added a comment - This new rule is called IncompatibleDependencyOverwrite , but I don't think that reflects the real purpose. A lower version can still be compatible. This rule is actually checking if it is using the highest of all defined versions per dependency. IMO something like ForceHighestDependencyVersion or ForceUpperBoundDependency would be a better name. WDYT?
        Hide
        Paul Gier added a comment -

        I agree the goal name could be more clear. How about RequireUpperBoundDeps? Since other standard rules use "Require" instead of "Force".

        Show
        Paul Gier added a comment - I agree the goal name could be more clear. How about RequireUpperBoundDeps ? Since other standard rules use "Require" instead of "Force".
        Hide
        Robert Scholte added a comment -

        You have my +1 for RequireUpperBoundDeps. I noticed Deps is already used for requireReleaseDeps, so that should be fine.

        Show
        Robert Scholte added a comment - You have my +1 for RequireUpperBoundDeps . I noticed Deps is already used for requireReleaseDeps , so that should be fine.
        Hide
        Paul Gier added a comment -

        Updated goal name in r1243269 and r1243270

        Show
        Paul Gier added a comment - Updated goal name in r1243269 and r1243270
        Hide
        Geoffrey De Smet added a comment - - edited

        I am ok with any name change,
        but I do think that "RequireHighestDependencyVersion" is simpler and clearer then "RequireUpperBoundDependencies".
        The term "Upper bound" might not be standard knowledge for the average programmer: http://en.wikipedia.org/wiki/Upper_and_lower_bounds

        Show
        Geoffrey De Smet added a comment - - edited I am ok with any name change, but I do think that "RequireHighestDependencyVersion" is simpler and clearer then "RequireUpperBoundDependencies". The term "Upper bound" might not be standard knowledge for the average programmer: http://en.wikipedia.org/wiki/Upper_and_lower_bounds
        Hide
        Paul Gier added a comment -

        The reason I didn't go with something like RequireHighestDependencyVersion is because it sounds like it will require the highest version available in the repository. Upper bound makes more sense to me because what you are saying is that the version in the POM is the highest version that is acceptable in the dependency tree.

        Anyway, I think as long at the description in the site docs are good, users will be able to figure out what it means.

        Show
        Paul Gier added a comment - The reason I didn't go with something like RequireHighestDependencyVersion is because it sounds like it will require the highest version available in the repository. Upper bound makes more sense to me because what you are saying is that the version in the POM is the highest version that is acceptable in the dependency tree. Anyway, I think as long at the description in the site docs are good, users will be able to figure out what it means.
        Hide
        Geoffrey De Smet added a comment -

        Ok, sounds good

        Show
        Geoffrey De Smet added a comment - Ok, sounds good
        Hide
        Paul Gier added a comment -

        I added a bit more description to the site docs, just to try to make this clear.
        r1243555

        Show
        Paul Gier added a comment - I added a bit more description to the site docs, just to try to make this clear. r1243555

          People

          • Assignee:
            Paul Gier
            Reporter:
            Geoffrey De Smet
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: