Maven 2 & 3
  1. Maven 2 & 3
  2. MNG-4275

[regression] Direct relocations no longer log at WARNING level : MNG-3380 conflicts with MNG-1689

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.0
    • Fix Version/s: 2.2.1
    • Component/s: Dependencies
    • Labels:
      None
    • Environment:
    • Complexity:
      Intermediate
    • Patch Submitted:
      Yes
    • Number of attachments :
      1

      Description

      Changes for MNG-3380 [1] to Process relocations before child-nodes are discovered during artifact collection, conflict with MNG-1689 [2] " Only print relocation warnings in standard output for the current pom". This results in a regression where (direct) relocations are no longer logged at WARNING level and are only logged at DEBUG. Direct relocations should be logged at WARNING level.

      @675087 [3] MNG-3380 was applied to DefaultArtifactCollector - the result is that the call to MavenMetadataSource#retrieveRelocatedArtifact() (then retrieveRelocatedProject()) occur before the call to artifact.setDependencyTrail( node.getDependencyTrail() ); in DefaultArtifactCollector. This results in a null list in MavenMetadataSource, which then fails the if-test to log at WARNING level introduced in MNG-1689.

      With a quick inspection I couldn't see the harm in bringing forward the call to:

      artifact.setDependencyTrail( node.getDependencyTrail() )

      , it is already called once when about to throw an exception, and this call can be replaced. Proposed patch makes the setDependencyTrail call earlier, prior to relocation detection.

      See also Nabble post [4].

      [1] http://jira.codehaus.org/browse/MNG-3380
      [2] http://jira.codehaus.org/browse/MNG-1689
      [3] http://svn.apache.org/viewvc?view=rev&revision=675087
      [4] http://www.nabble.com/2.0.9-%3E2.1.0-change-regression-in-relocation-WARNING--td24368186.html

        Activity

        Hide
        Brett Randall added a comment -

        Proposed patch, making call to artifact.setDependencyTrail( node.getDependencyTrail() ); earlier.

        Show
        Brett Randall added a comment - Proposed patch, making call to artifact.setDependencyTrail( node.getDependencyTrail() ); earlier.
        Hide
        John Casey added a comment -

        This needs a test case before we can apply the patch.

        Show
        John Casey added a comment - This needs a test case before we can apply the patch.
        Hide
        Brett Randall added a comment -

        OK - in order to write the test case, since we are testing log-invocation on a relocated artifact, I'll need to:

        1) Construct a relocated artifact in the test, and
        2) Create a mock Log so that we can test if it is called with warn or debug.

        I'm not familiar with the existing test-util assets in this area - both might require a bit of work.

        Show
        Brett Randall added a comment - OK - in order to write the test case, since we are testing log-invocation on a relocated artifact, I'll need to: 1) Construct a relocated artifact in the test, and 2) Create a mock Log so that we can test if it is called with warn or debug. I'm not familiar with the existing test-util assets in this area - both might require a bit of work.
        Hide
        John Casey added a comment -

        I'll take a crack at the test case today.

        Show
        John Casey added a comment - I'll take a crack at the test case today.
        Hide
        John Casey added a comment -

        got a test case put together, but I'm having some connectivity problems with SVN, so I'll commit it to the core-integration-testing trunk as soon as I can.

        Show
        John Casey added a comment - got a test case put together, but I'm having some connectivity problems with SVN, so I'll commit it to the core-integration-testing trunk as soon as I can.
        Hide
        John Casey added a comment -

        committed integration test, and applied patch.

        Thanks, Brett.

        Show
        John Casey added a comment - committed integration test, and applied patch. Thanks, Brett.

          People

          • Assignee:
            John Casey
            Reporter:
            Brett Randall
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: