Maven 2 & 3

[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

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: