Hi Karl Heinz,

So I went ahead and created https://issues.apache.org/jira/browse/MSHARED-596 for this. The ticket is about adding "getPathForLocalMetadata" on the RepositoryManager. In turn, this would simplify DefaultProjectInstaller. I attached a patch to it but would like some comments before applying it.


Thanks,
Guillaume

Le 09/10/2016 à 15:01, Karl Heinz Marbaise a écrit :
Hi Guillaume,

This is already used in other locations of the maven-artifact-transfer...

Maven30ArtifactRepositoryAdapter, Maven31ArtifactRepositoryAdapter, Maven30ArtifactDeployer, Maven31ArtifactDeployer...only to mention a few...

org.apache.maven.repository.legacy.metadata.ArtifactMetadata is not deprecated and the interface


public interface ArtifactMetadata
    extends org.apache.maven.repository.legacy.metadata.ArtifactMetadata
{
    void merge( ArtifactMetadata metadata );
}

So the question is simply if we need the merge method? But I'm not sure if we should go to legacy area here?

Someone who has a better suggestion/alternatives?


Kind regards
Karl Heinz


On 09/10/16 14:54, Guillaume Boué wrote:
The only quirk is that
org.apache.maven.artifact.metadata.ArtifactMetadata is deprecated... so
this would add usage of the deprecated interface in RepositoryManager.
What is the alternative for this?


Le 09/10/2016 à 14:36, Karl Heinz Marbaise a écrit :
Hi Buillaume,

On 09/10/16 14:28, Guillaume Boué wrote:
If the caller needs to force a different local repository than the one
in the Maven session, they can already call
"repositoryManager.setLocalRepositoryBasedir" on the building request
that is passed to ProjectInstaller. This is what the plugins using
ArtifactInstaller are already doing, and it would be simpler than
creating a whole new MavenArtifactRepository and passing that. Maybe
this ArtifactRepository parameter could even be removed?

For now, its current use by the ProjectInstaller is only to get the path
to the metadata to install checksums for them, and this is only needed
because there is no "getPathForLocalMetadata" on the RepositoryManager.
How about adding this method, that would delegate to proper
implementation classes for Maven 3.1 or 3.0?

Sounds better and cleaner...


Kind regards
Karl Heinz




Le 09/10/2016 à 11:31, Karl Heinz Marbaise a écrit :
Hi,

Yes it has been introduced by me...and yes not all plugins are using
it cause I would like to have some tests before spreading it to the
"world" of plugins..

Kind regards
Karl Heinz


On 09/10/16 11:07, Robert Scholte wrote:
Hi,

IIRC ProjectInstaller has been introduced recently, I can imagine it
hasn't been pushed to all plugins which should use it instead of the
ArtifactInstaller.
While implementing maven-artifact-transfer and using it in several
plugins I just hit these edge cases where the local repo is not always
the target repo.

Robert

On Sat, 08 Oct 2016 22:30:11 +0200, Guillaume Boué <gb...@apache.org>
wrote:

Hi,

From what I checked, I don't think those plugins should be impacted
since they use the ArtifactInstaller directly, and not the
ProjectInstaller.

But I can add an overload taking an ArtifactRepository which would
get
the path to the artifact with "artifactRepository.pathOf(artifact)".
And then go with the one using the RepositoryManager or not,
depending
on whether the ArtifactRepository is null or not.

Guillaume


Le 08/10/2016 à 22:11, Robert Scholte a écrit :
Hi Guillaume,

although this is often true, there are some plugins which create
their own local repository, for instance maven-invoker-plugin and
maven-dependency-plugin. In those cases you should pass the
ArtifactRepository.
So we will need those versions too, either as overloaded method or
restored where artifactRepository can be null.

thanks,
Robert


On Sat, 08 Oct 2016 20:43:58 +0200, <gb...@apache.org> wrote:

Author: gboue
Date: Sat Oct  8 18:43:58 2016
New Revision: 1763929

URL: http://svn.apache.org/viewvc?rev=1763929&view=rev
Log:
[MSHARED-595] In DefaultProjectInstaller, the path to the local
repository should be retrieved from the RepositoryManager

We need to rely on the RepositoryManager to get a hold of the local
repository base directory.

Modified:
maven/shared/trunk/maven-artifact-transfer/src/main/java/org/apache/maven/shared/project/install/internal/DefaultProjectInstaller.java




Modified:
maven/shared/trunk/maven-artifact-transfer/src/main/java/org/apache/maven/shared/project/install/internal/DefaultProjectInstaller.java



URL:
http://svn.apache.org/viewvc/maven/shared/trunk/maven-artifact-transfer/src/main/java/org/apache/maven/shared/project/install/internal/DefaultProjectInstaller.java?rev=1763929&r1=1763928&r2=1763929&view=diff



==============================================================================


---
maven/shared/trunk/maven-artifact-transfer/src/main/java/org/apache/maven/shared/project/install/internal/DefaultProjectInstaller.java


(original)
+++
maven/shared/trunk/maven-artifact-transfer/src/main/java/org/apache/maven/shared/project/install/internal/DefaultProjectInstaller.java


Sat Oct  8 18:43:58 2016
@@ -101,7 +101,7 @@ public class DefaultProjectInstaller
             {
                 installer.install( buildingRequest,
Collections.<Artifact>singletonList( new ProjectArtifact( project )
) );
-                installChecksums( buildingRequest,
artifactRepository, artifact, createChecksum );
+                installChecksums( buildingRequest, artifact,
createChecksum );
                 addMetaDataFilesForArtifact( artifactRepository,
artifact, metadataFiles, createChecksum );
             }
         }
@@ -120,7 +120,7 @@ public class DefaultProjectInstaller
             if ( file != null && file.isFile() )
             {
                 installer.install( buildingRequest,
Collections.<Artifact>singletonList( artifact ) );
-                installChecksums( buildingRequest,
artifactRepository, artifact, createChecksum );
+                installChecksums( buildingRequest, artifact,
createChecksum );
                 addMetaDataFilesForArtifact( artifactRepository,
artifact, metadataFiles, createChecksum );
             }
             else if ( !attachedArtifacts.isEmpty() )
@@ -139,7 +139,7 @@ public class DefaultProjectInstaller
         for ( Artifact attached : attachedArtifacts )
         {
             installer.install( buildingRequest,
Collections.singletonList( attached ) );
- installChecksums( buildingRequest, artifactRepository,
attached, createChecksum );
+            installChecksums( buildingRequest, attached,
createChecksum );
             addMetaDataFilesForArtifact( artifactRepository,
attached, metadataFiles, createChecksum );
         }
@@ -153,12 +153,12 @@ public class DefaultProjectInstaller
      * the original POM file (cf. MNG-2820). While the plugin
currently requires Maven 2.0.6, we continue to hash the
* installed POM for robustness with regard to future changes
like re-introducing some kind of POM filtering.
      *
+     * @param buildingRequest The project building request, must
not be <code>null</code>.
* @param artifact The artifact for which to create checksums,
must not be <code>null</code>.
      * @param createChecksum {@code true} if checksum should be
created, otherwise {@code false}.
      * @throws IOException If the checksums could not be
installed.
      */
-    private void installChecksums( ProjectBuildingRequest
buildingRequest, ArtifactRepository artifactRepository,
-                                   Artifact artifact, boolean
createChecksum )
+    private void installChecksums( ProjectBuildingRequest
buildingRequest, Artifact artifact, boolean createChecksum )
         throws IOException
     {
         if ( !createChecksum )
@@ -166,7 +166,7 @@ public class DefaultProjectInstaller
             return;
         }
-        File artifactFile = getLocalRepoFile( buildingRequest,
artifactRepository, artifact );
+        File artifactFile = getLocalRepoFile( buildingRequest,
artifact );
         installChecksums( artifactFile );
     }
@@ -257,14 +257,14 @@ public class DefaultProjectInstaller
      * Gets the path of the specified artifact within the local
repository. Note that the returned path need not exist
      * (yet).
      *
+     * @param buildingRequest The project building request, must
not be <code>null</code>.
      * @param artifact The artifact whose local repo path
should be
determined, must not be <code>null</code>.
      * @return The absolute path to the artifact when installed,
never <code>null</code>.
      */
-    private File getLocalRepoFile( ProjectBuildingRequest
buildingRequest, ArtifactRepository artifactRepository,
-                                   Artifact artifact )
+    private File getLocalRepoFile( ProjectBuildingRequest
buildingRequest, Artifact artifact )
     {
         String path = repositoryManager.getPathForLocalArtifact(
buildingRequest, artifact );
- return new File( artifactRepository.getBasedir(), path );
+        return new File(
repositoryManager.getLocalRepositoryBasedir( buildingRequest ),
path );
     }
    /**


---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org



---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

Reply via email to