Maven 2 & 3

Maven not picking up specific timestamped version dependency when a later timestamped version was downloaded and already present in the local repository

Details

  • Complexity:
    Intermediate
  • Number of attachments :
    4

Description

To reproduce this issue:

  1. Create a project (let's call this projectA) with a class named ClassA having a method named methodA(). Set the version as 1.0-SNAPSHOT and set uniqueVersion=true.
  2. Deploy this in a remote repository
  3. Create another project (let's call this projectB) which has a dependency on projectA. Set the dependency's version to the specific timestamped version when projectA was deployed in step 2. Create a class named ClassB and add a method which invokes ClassA's methodA().
  4. Add your remote repository either in the settings or in the pom.
  5. Build projectB. You will get a successful build.
  6. Now go back to projectA and remove methodA() from classA.
  7. Deploy projectA to the remote repository again.
  8. Update the dependency version of projectA in projectB's pom.xml. Set it to the latest timestamp version.
  9. Build projectB. Your build will fail because methodA() was removed.
  10. Revert the dependency version of projectA in projectB's pom.xml. Set it to the same value you've set in step 3.
  11. Build projectB. Your build will still fail even though you've set the correct version.
  1. MNG-4189.patch
    11/Jun/09 7:08 AM
    2 kB
    Benjamin Bentmann
  2. MNG-4189-core-integration-testing.patch
    09/Jun/09 5:51 AM
    21 kB
    Maria Odea Ching
  3. MNG-4189-maven-2.1.x.patch
    09/Jun/09 5:56 AM
    7 kB
    Maria Odea Ching

Activity

Hide
Maria Odea Ching added a comment -

I'm still creating an IT for this, will attach it to this jira issue once completed..

Show
Maria Odea Ching added a comment - I'm still creating an IT for this, will attach it to this jira issue once completed..
Hide
Maria Odea Ching added a comment -

Attaching IT for this bug..

Show
Maria Odea Ching added a comment - Attaching IT for this bug..
Hide
Maria Odea Ching added a comment -

Attaching patch for core-integration-testing integrating the IT test for this issue..

Show
Maria Odea Ching added a comment - Attaching patch for core-integration-testing integrating the IT test for this issue..
Hide
Maria Odea Ching added a comment - - edited

Attaching MNG-4189-maven-2.1.x.patch to fix this issue. Unit tests included in the patch. Could someone please review this before I commit it to 2.1.x branch? Thanks in advance!

Show
Maria Odea Ching added a comment - - edited Attaching MNG-4189-maven-2.1.x.patch to fix this issue. Unit tests included in the patch. Could someone please review this before I commit it to 2.1.x branch? Thanks in advance!
Hide
Benjamin Bentmann added a comment -

With your patch, we effectively have

boolean resolved = false;
if ( !destination.exists() || force )
{
    ...
    resolved = true;
}
else if ( destination.exists() )     // Note that this is equivalent to just "else"
{
    ...
    resolved = true;
}

In other words, the flag resolved will always be true. This in turn will make if ( resolved || !copy.exists() ) equivalent to if (true), thereby always executing the FileUtils.copyFile(destination, copy). Hence, Maven will end up copying each and every snapshot dependency. For bigger projects, I can imagine this accumulates to significant IO, slowing down the build.

The attached patch proposes to compare the file's timestamp&size instead of using the resolved flag. This should also ensure that artifact-<baseVersion>.jar matches the last resolved timestamp version but still avoids unnecessary file copies in case nothing changed.

Show
Benjamin Bentmann added a comment - With your patch, we effectively have
boolean resolved = false;
if ( !destination.exists() || force )
{
    ...
    resolved = true;
}
else if ( destination.exists() )     // Note that this is equivalent to just "else"
{
    ...
    resolved = true;
}
In other words, the flag resolved will always be true. This in turn will make if ( resolved || !copy.exists() ) equivalent to if (true), thereby always executing the FileUtils.copyFile(destination, copy). Hence, Maven will end up copying each and every snapshot dependency. For bigger projects, I can imagine this accumulates to significant IO, slowing down the build. The attached patch proposes to compare the file's timestamp&size instead of using the resolved flag. This should also ensure that artifact-<baseVersion>.jar matches the last resolved timestamp version but still avoids unnecessary file copies in case nothing changed.
Hide
Benjamin Bentmann added a comment -

A comment about the IT: I suggest to remove the checksum files (*.md5, *.sha1) from the test repository and just setup the repo to ignore the checksums. Files that are not vital for a test otherwise simply cause IO and slow down resource copying/extraction during testing. This is not relevant for a single test but we approach 400 ITs so there's a certain scaling factor to consider.

Show
Benjamin Bentmann added a comment - A comment about the IT: I suggest to remove the checksum files (*.md5, *.sha1) from the test repository and just setup the repo to ignore the checksums. Files that are not vital for a test otherwise simply cause IO and slow down resource copying/extraction during testing. This is not relevant for a single test but we approach 400 ITs so there's a certain scaling factor to consider.
Hide
Maria Odea Ching added a comment -

Ok, thanks for reviewing them Benjamin. I'll update the patches. Should I merge this to 2.2.x branch as well after I commit it to 2.1.x branch?

Show
Maria Odea Ching added a comment - Ok, thanks for reviewing them Benjamin. I'll update the patches. Should I merge this to 2.2.x branch as well after I commit it to 2.1.x branch?
Hide
Benjamin Bentmann added a comment -

Should I merge this to 2.2.x branch as well

Yes, please, I don't see a reason to not do so.

Show
Benjamin Bentmann added a comment -
Should I merge this to 2.2.x branch as well
Yes, please, I don't see a reason to not do so.
Hide
Maria Odea Ching added a comment -

Fix committed in 2.1.x branch -r788583 and merged to 2.2.x branch -r788596.
Integration tests committed in core-integration-testing trunk -r788584.

Show
Maria Odea Ching added a comment - Fix committed in 2.1.x branch -r788583 and merged to 2.2.x branch -r788596. Integration tests committed in core-integration-testing trunk -r788584.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: