Maven Dependency Plugin
  1. Maven Dependency Plugin
  2. MDEP-294

copy-dependencies goal doesn't properly respect classifier when creating base version of snapshots

    Details

    • Type: Bug Bug
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.1
    • Fix Version/s: None
    • Component/s: copy-dependencies
    • Labels:
      None
    • Patch Submitted:
      Yes
    • Number of attachments :
      4

      Description

      CopyDependenciesMojo ignores any classifier on the artifact being copied when creating the base version of a snapshot. It works correctly for the non-base (timestamped) version. This leads to a mismatch in the copied dependencies where the timestamped version correctly keeps the classifier, but the base -SNAPSHOT version has the classifier completely dropped.

      The fix is simple, although a bit ugly. In the installBaseSnapshot method, a check must be made for the presence of a classifier on the artifact being copied before using the ArtifactFactory to create a copy of the base version. Ideally, the ArtifactFactory would expose a single method that takes all parameters, but unfortunately it does not. This requires a separate 'if' check for the presence of a classifier.

      Another potential issue is that the method ArtifactFactory#createArtifactWithClassifier has no parameter for scope. I don't think that causes any issue in this case, but is another reason why there should be a single method createArtifact that takes all combinations of parameters including classifier.

      I've attached a patch that will fix the issue, but not a test case since it looks like the maven-plugin-testing-tools-harness would need to be updated as well. It doesn't appear to expose any artifacts that both have a classifier and are snapshots from the ArtifactStubFactory. If deemed important, I can produce a patch for that as well along with a test.

      1. CopyDependenciesMojo.java
        8 kB
        Tim Downey
      2. CopyDependenciesMojo.java.diff
        1 kB
        Tim Downey
      3. TestCopyDependenciesMojo.java
        25 kB
        Tim Downey
      4. TestCopyDependenciesMojo2.diff
        4 kB
        Tim Downey

        Activity

        Hide
        Brian Fox added a comment -

        Can you add a test?

        Show
        Brian Fox added a comment - Can you add a test?
        Hide
        Tim Downey added a comment -

        It looks like the artifacts used in the test cases come from the maven-plugin-testing-tools-harness (The artifacts supplied by the current harness don't have any that use a classifier and are snapshot). If so, I'd need to modify that as well, right? Would I submit a patch for both to this ticket?

        Show
        Tim Downey added a comment - It looks like the artifacts used in the test cases come from the maven-plugin-testing-tools-harness (The artifacts supplied by the current harness don't have any that use a classifier and are snapshot). If so, I'd need to modify that as well, right? Would I submit a patch for both to this ticket?
        Hide
        Tim Downey added a comment -

        I'm attaching a patch to TestCopyDependenciesMojo2 that exercises the fix. It adds an expanded snapshot artifact including a classifier to the testRepositoryLayout test.

        Show
        Tim Downey added a comment - I'm attaching a patch to TestCopyDependenciesMojo2 that exercises the fix. It adds an expanded snapshot artifact including a classifier to the testRepositoryLayout test.
        Hide
        Tim Downey added a comment -

        Any reason that this wasn't released with 2.2 or can't be scheduled for 2.3? My patch contains tests, so I was wondering if I missed something or this is some other unexpected breakage that my patch may cause.

        Show
        Tim Downey added a comment - Any reason that this wasn't released with 2.2 or can't be scheduled for 2.3? My patch contains tests, so I was wondering if I missed something or this is some other unexpected breakage that my patch may cause.

          People

          • Assignee:
            Unassigned
            Reporter:
            Tim Downey
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: