Maven Project Info Reports Plugin
  1. Maven Project Info Reports Plugin
  2. MPIR-238

Dependency File Details section of the dependencies report shows 'target/classes' for reactor artifacts.

    Details

    • Type: Bug Bug
    • Status: Reopened
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.4
    • Fix Version/s: None
    • Component/s: dependency-info
    • Labels:
      None

      Description

      Generating the dependencies report in a multi-module project leads to incorrect entries in the 'Dependency File Details' section of the dependencies report. For example, the Maven Release Plugin Dependency File Details report contains the following entry:

      Filename Size Entries Classes Packages JDK Rev Debug
      maven-release-manager/target/classes - 0 0 0 - release

      Building the site of a single module ('mvn site' in that modules directory), the correct entries are shown.

        Issue Links

          Activity

          Christian Schulte created issue -
          Hide
          Christian Schulte added a comment - - edited

          The cause is class 'org.apache.maven.ReactorReader' falling back to returning directories when the 'compile' phase has completed which is the case for various report plugins marked '@execute phase="generate-test-sources"'. MPIR-238.patch contains a patch for the dependencies report adding '@execute phase="package"' to make the reactor reader stop returning directories. MPIR-238-ReactorReader.patch contains a patch for that class to not resolve to directories when not executing the default lifecycle.

          Show
          Christian Schulte added a comment - - edited The cause is class 'org.apache.maven.ReactorReader' falling back to returning directories when the 'compile' phase has completed which is the case for various report plugins marked '@execute phase="generate-test-sources"'. MPIR-238 .patch contains a patch for the dependencies report adding '@execute phase="package"' to make the reactor reader stop returning directories. MPIR-238 -ReactorReader.patch contains a patch for that class to not resolve to directories when not executing the default lifecycle.
          schulte2005 made changes -
          Field Original Value New Value
          Attachment MPIR-238.patch [ 58333 ]
          Attachment MPIR-238-ReactorReader.patch [ 58334 ]
          Hide
          Christian Schulte added a comment -

          This also affects the 'Dependency Repository Locations' report, which does not link to snapshot repositories in such situations.

          Show
          Christian Schulte added a comment - This also affects the 'Dependency Repository Locations' report, which does not link to snapshot repositories in such situations.
          Hide
          Christian Schulte added a comment -

          Updated patch to test for the 'pre-clean' phase instead of 'clean'.

          Show
          Christian Schulte added a comment - Updated patch to test for the 'pre-clean' phase instead of 'clean'.
          schulte2005 made changes -
          Attachment MPIR-238-ReactorReader.patch [ 58335 ]
          Michael Osipov made changes -
          Status Open [ 1 ] Closed [ 6 ]
          Resolution Won't Fix [ 2 ]
          Hide
          Christian Schulte added a comment - - edited

          I think this issue should be taken care of. It maybe should be filed for Maven core since the cause of this issue really is a change in behaviour between Maven 2 and Maven 3. To summarize: In Maven 3, the core got changed to reference class files from e.g. the 'target/classes' directories in certain cases. This change stops maven from downloading artifacts from remote repositories. I think that method hasBeenPackaged of class ReactorReader should also check for the artifact to be available in some repository instead of just checking the package phase. The change I am requesting is to update method hasBeenPackaged in a way that it returns true, if the artifact being checked is found in a remote repository. For example:

          private boolean hasBeenPackaged( MavenProject project )
          {
            return project.hasLifecyclePhase( "package" )
                || project.hasLifecyclePhase( "install" )
                || project.hasLifecyclePhase( "deploy" )
                || project artifact can be resolved from a remote repository.
          }
          
          Show
          Christian Schulte added a comment - - edited I think this issue should be taken care of. It maybe should be filed for Maven core since the cause of this issue really is a change in behaviour between Maven 2 and Maven 3. To summarize: In Maven 3, the core got changed to reference class files from e.g. the 'target/classes' directories in certain cases. This change stops maven from downloading artifacts from remote repositories. I think that method hasBeenPackaged of class ReactorReader should also check for the artifact to be available in some repository instead of just checking the package phase. The change I am requesting is to update method hasBeenPackaged in a way that it returns true , if the artifact being checked is found in a remote repository. For example: private boolean hasBeenPackaged( MavenProject project ) { return project.hasLifecyclePhase( " package " ) || project.hasLifecyclePhase( "install" ) || project.hasLifecyclePhase( "deploy" ) || project artifact can be resolved from a remote repository. }
          schulte2005 made changes -
          Status Closed [ 6 ] Reopened [ 4 ]
          Resolution Won't Fix [ 2 ]
          Michael Osipov made changes -
          Comment [ Please refer to https://cwiki.apache.org/confluence/display/MAVEN/The+Great+JIRA+Cleanup+of+2014 if you're wondering why this issue was closed out. ]
          Hide
          Michael Osipov added a comment -

          Christian, is the patch still valid? If yes, retest please otherwise delete it right away.

          Show
          Michael Osipov added a comment - Christian, is the patch still valid? If yes, retest please otherwise delete it right away.
          Hide
          Christian Schulte added a comment -

          I am going to delete the patches. This issue needs more discussion. In Maven 3, the core got changed to fall back to loose class files from target/classes directories to solve various issues. However, this leads to issues with projects where the content of the target/classes directory does not match the content of e.g. the jar file the project produces. A project using the maven-shade-plugin may produce a jar with totally different content than the content of the target/classes directory. The maven-jar-plugin may be configured to exclude various files from the target/classes directory. The clirr-maven-plugin must not compare the content of the target/classes directory with the jar of the previous version etc. My suggestion is to fall back to loose class files only if falling back really is the only way to not fail the build. If the packaged artifact is available, there is no need to fall back to anything, in my opinion. Also, I don't think hasLifecyclePhase should be backing this decision since you cannot tell how a project performs its packaging.

          Show
          Christian Schulte added a comment - I am going to delete the patches. This issue needs more discussion. In Maven 3, the core got changed to fall back to loose class files from target/classes directories to solve various issues. However, this leads to issues with projects where the content of the target/classes directory does not match the content of e.g. the jar file the project produces. A project using the maven-shade-plugin may produce a jar with totally different content than the content of the target/classes directory. The maven-jar-plugin may be configured to exclude various files from the target/classes directory. The clirr-maven-plugin must not compare the content of the target/classes directory with the jar of the previous version etc. My suggestion is to fall back to loose class files only if falling back really is the only way to not fail the build. If the packaged artifact is available, there is no need to fall back to anything, in my opinion. Also, I don't think hasLifecyclePhase should be backing this decision since you cannot tell how a project performs its packaging.
          schulte2005 made changes -
          Attachment MPIR-238.patch [ 58333 ]
          schulte2005 made changes -
          Attachment MPIR-238-ReactorReader.patch [ 58335 ]
          schulte2005 made changes -
          Attachment MPIR-238-ReactorReader.patch [ 58334 ]
          Hide
          Michael Osipov added a comment -

          Christian, check MPIR-322 please.

          Show
          Michael Osipov added a comment - Christian, check MPIR-322 please.
          Michael Osipov made changes -
          Link This issue is related to MPIR-322 [ MPIR-322 ]
          Hide
          Christian Schulte added a comment -

          Thanks for bringing this up. The solution to MPIR-322 as it got committed is incorrect in my opinion. Searching for a jar file in target is not what should be done. Resolution is to take place solely in the core. Issues with the solution to MPIR-322 are things like:

          mvn package|install|deploy clean site

          No jar in target will be found. The same problem exists for artifacts not installed locally. For example, checking out some multi-module project release version and just executing mvn site needs to make Maven 3 download the deployed artifacts of the modules from the remote repositories. Exactly the same way Maven 2 behaves. There are use cases where falling back to loose class files isn't wanted/needed/expected. I am thinking about introducing a new command line flag similar to the legacy-local-repository. Maybe legacy-reactor-resolution. I could provide a patch doing this. However, there would need to be some consensus on how to solve MPIR-322 and MPIR-238 properly.

          Show
          Christian Schulte added a comment - Thanks for bringing this up. The solution to MPIR-322 as it got committed is incorrect in my opinion. Searching for a jar file in target is not what should be done. Resolution is to take place solely in the core. Issues with the solution to MPIR-322 are things like: mvn package|install|deploy clean site No jar in target will be found. The same problem exists for artifacts not installed locally. For example, checking out some multi-module project release version and just executing mvn site needs to make Maven 3 download the deployed artifacts of the modules from the remote repositories. Exactly the same way Maven 2 behaves. There are use cases where falling back to loose class files isn't wanted/needed/expected. I am thinking about introducing a new command line flag similar to the legacy-local-repository . Maybe legacy-reactor-resolution . I could provide a patch doing this. However, there would need to be some consensus on how to solve MPIR-322 and MPIR-238 properly.
          Hide
          Hervé Boutemy added a comment - - edited

          The solution to MPIR-322 as it got committed is incorrect in my opinion

          +1, it's not the perfect and most generic solution

          things like mvn package|install|deploy clean site

          cleaning after compile then generating site is just a proof of the "it's not ideal" concept: but who wants to do that, not only to show that the MPIR-322 solution is only a workaround for core limitations?

          The same problem exists for artifacts not installed locally

          +1: I never said MPIR-322 is perfect
          a plugin can't fix a core issue: it only can sometime workaround it, like the great idea in MPIR-247 after fixing MNG-5568

          the more I work on m-site-p (and I really mean work on it), the more I see the limitations we have in Maven core for really supporting a lot of things such a plugin require, ie coordination of plugins in a lifecycle parallel to the standard build lifecycle to summarize the whole idea

          But m-site-p works not so bad for a long time

          Then even if I know that there are strong limitations that will require strong changes in Maven core, we'll have to live with limited workarounds to extend the "m-site-p works not so bad for a long time" mantra as much as possible instead of focusing only on "there are strong limitations that will require strong changes in Maven core"

          Show
          Hervé Boutemy added a comment - - edited The solution to MPIR-322 as it got committed is incorrect in my opinion +1, it's not the perfect and most generic solution things like mvn package|install|deploy clean site cleaning after compile then generating site is just a proof of the "it's not ideal" concept: but who wants to do that, not only to show that the MPIR-322 solution is only a workaround for core limitations? The same problem exists for artifacts not installed locally +1: I never said MPIR-322 is perfect a plugin can't fix a core issue: it only can sometime workaround it, like the great idea in MPIR-247 after fixing MNG-5568 the more I work on m-site-p (and I really mean work on it), the more I see the limitations we have in Maven core for really supporting a lot of things such a plugin require, ie coordination of plugins in a lifecycle parallel to the standard build lifecycle to summarize the whole idea But m-site-p works not so bad for a long time Then even if I know that there are strong limitations that will require strong changes in Maven core, we'll have to live with limited workarounds to extend the "m-site-p works not so bad for a long time" mantra as much as possible instead of focusing only on "there are strong limitations that will require strong changes in Maven core"
          Hide
          Christian Schulte added a comment -

          There are ways to solve this without having to change the core and without having to work around anything. Just updating the reports expecting a packaged archive to @execute phase="package" will already make the core resolve to the jar instead of target/classes as expected. Meanwhile I've opened a pull request https://github.com/apache/maven/pull/32 to add the option mentioned above.

          Show
          Christian Schulte added a comment - There are ways to solve this without having to change the core and without having to work around anything. Just updating the reports expecting a packaged archive to @execute phase="package" will already make the core resolve to the jar instead of target/classes as expected. Meanwhile I've opened a pull request https://github.com/apache/maven/pull/32 to add the option mentioned above.
          Hide
          Hervé Boutemy added a comment -

          Just updating the reports expecting a packaged archive to @execute phase="package" will already make the core resolve to the jar instead of target/classes as expected

          I see the idea
          I also know the problems with such a solution: see MJAVADOC-171 which is why reporting plugins which used to have such execution finally add new "-nofork" goals

          for https://github.com/apache/maven/pull/32, we need a MNG issue since it's a core fix
          and it will require core IT and discussions on dev@ IMHO: I think such a discussion will be useful but can be complex (since adding an option is just a workaround too, then I don't think it will be pleasant for everybody )

          I don't have time to do it at the moment, but I'm interested to work with other in january on that (which is a can of worms that we'll need to open one day or the other)

          Show
          Hervé Boutemy added a comment - Just updating the reports expecting a packaged archive to @execute phase="package" will already make the core resolve to the jar instead of target/classes as expected I see the idea I also know the problems with such a solution: see MJAVADOC-171 which is why reporting plugins which used to have such execution finally add new "-nofork" goals for https://github.com/apache/maven/pull/32 , we need a MNG issue since it's a core fix and it will require core IT and discussions on dev@ IMHO: I think such a discussion will be useful but can be complex (since adding an option is just a workaround too, then I don't think it will be pleasant for everybody ) I don't have time to do it at the moment, but I'm interested to work with other in january on that (which is a can of worms that we'll need to open one day or the other)
          Hide
          Christian Schulte added a comment -

          (since adding an option is just a workaround too, then I don't think it will be pleasant for everybody )

          Absolutely. Some enhancements in Maven 3 made it misbehave in certain cases. An option to revert those enhancements temporarily is a work around for this. Retains backwards compatibility. I will open a new core issue for this.

          Show
          Christian Schulte added a comment - (since adding an option is just a workaround too, then I don't think it will be pleasant for everybody ) Absolutely. Some enhancements in Maven 3 made it misbehave in certain cases. An option to revert those enhancements temporarily is a work around for this. Retains backwards compatibility. I will open a new core issue for this.
          schulte2005 made changes -
          Link This issue is superceded by MNG-5738 [ MNG-5738 ]
          Hide
          Hervé Boutemy added a comment -

          thank you Christian: I'll work on MNG-5738 in early january (if nobody beats me at it)

          Show
          Hervé Boutemy added a comment - thank you Christian: I'll work on MNG-5738 in early january (if nobody beats me at it)
          Mark Thomas made changes -
          Project Import Sun Apr 05 12:10:01 UTC 2015 [ 1428235801085 ]
          Mark Thomas made changes -
          Workflow jira [ 12723500 ] Default workflow, editable Closed status [ 12755830 ]
          Mark Thomas made changes -
          Project Import Mon Apr 06 00:45:01 UTC 2015 [ 1428281101090 ]
          Mark Thomas made changes -
          Workflow jira [ 12960980 ] Default workflow, editable Closed status [ 12997770 ]
          Hide
          Michael Osipov added a comment -

          Hervé Boutemy, has this implicitly been resolved by MPIR-322?

          Show
          Michael Osipov added a comment - Hervé Boutemy , has this implicitly been resolved by MPIR-322 ?
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Closed Closed
          1063d 9h 24m 1 Michael Osipov 25/Nov/14 14:47
          Closed Closed Reopened Reopened
          11h 30m 1 schulte2005 26/Nov/14 02:18

            People

            • Assignee:
              Unassigned
              Reporter:
              Christian Schulte
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:

                Development