You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "cstamas (via GitHub)" <gi...@apache.org> on 2023/02/22 10:15:18 UTC

[GitHub] [maven-shade-plugin] cstamas opened a new pull request, #175: [MSHADE-438] Update to Maven 3.2.5

cstamas opened a new pull request, #175:
URL: https://github.com/apache/maven-shade-plugin/pull/175

   Up the baseline as for the rest of ASF plugins.
   
   Changes:
   * drop MAT
   * drop uses of archaic constructs
   
   Also update IT for MSHADE-340 as it was not passing with maven-jar-plugin 3+ version (maven 3.8.x used maven-jar-plugin 2.4), as the IT was replacing the main JAR intentionally, or unintentionally.
   
   ---
   
   https://issues.apache.org/jira/browse/MSHADE-438
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-shade-plugin] cstamas commented on a diff in pull request #175: [MSHADE-438] Update to Maven 3.2.5

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #175:
URL: https://github.com/apache/maven-shade-plugin/pull/175#discussion_r1114290727


##########
src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java:
##########
@@ -846,29 +833,29 @@ private void replaceFile( File oldFile, File newFile )
     private void copyFiles( File source, File target )
         throws IOException
     {
-        try ( InputStream in = new FileInputStream( source );
-              OutputStream out = new FileOutputStream( target ) )
+        try ( InputStream in = Files.newInputStream( source.toPath() );
+              OutputStream out = Files.newOutputStream( target.toPath() ) )
         {
             IOUtil.copy( in, out );
         }
     }
 
     private File resolveArtifactForClassifier( Artifact artifact, String classifier )
     {
-        DefaultArtifactCoordinate coordinate = new DefaultArtifactCoordinate();
-        coordinate.setGroupId( artifact.getGroupId() );
-        coordinate.setArtifactId( artifact.getArtifactId() );
-        coordinate.setVersion( artifact.getVersion() );
-        coordinate.setExtension( "jar" );
-        coordinate.setClassifier( classifier );
+        ArtifactType artifactType = session.getRepositorySession().getArtifactTypeRegistry().get( artifact.getType() );

Review Comment:
   This is NPE prone: i forgot that resolver ArtifactTypeRegistry is DIFFERENT that maven's ArtifactHandlerManager (never returns null).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-shade-plugin] cstamas merged pull request #175: [MSHADE-438] Update to Maven 3.2.5

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas merged PR #175:
URL: https://github.com/apache/maven-shade-plugin/pull/175


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-shade-plugin] cstamas commented on a diff in pull request #175: [MSHADE-438] Update to Maven 3.2.5

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #175:
URL: https://github.com/apache/maven-shade-plugin/pull/175#discussion_r1114292021


##########
src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java:
##########
@@ -846,29 +833,29 @@ private void replaceFile( File oldFile, File newFile )
     private void copyFiles( File source, File target )
         throws IOException
     {
-        try ( InputStream in = new FileInputStream( source );
-              OutputStream out = new FileOutputStream( target ) )
+        try ( InputStream in = Files.newInputStream( source.toPath() );
+              OutputStream out = Files.newOutputStream( target.toPath() ) )
         {
             IOUtil.copy( in, out );
         }
     }
 
     private File resolveArtifactForClassifier( Artifact artifact, String classifier )
     {
-        DefaultArtifactCoordinate coordinate = new DefaultArtifactCoordinate();
-        coordinate.setGroupId( artifact.getGroupId() );
-        coordinate.setArtifactId( artifact.getArtifactId() );
-        coordinate.setVersion( artifact.getVersion() );
-        coordinate.setExtension( "jar" );
-        coordinate.setClassifier( classifier );
+        ArtifactType artifactType = session.getRepositorySession().getArtifactTypeRegistry().get( artifact.getType() );
+        org.eclipse.aether.artifact.DefaultArtifact coordinate = new DefaultArtifact( artifact.getGroupId(),
+                artifact.getArtifactId(), classifier, artifactType.getExtension(), artifact.getVersion(),

Review Comment:
   actually this `artifactType.getExtension()` is NPE prone, as artifactType can be null



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-shade-plugin] cstamas commented on pull request #175: [MSHADE-438] Update to Maven 3.2.5

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #175:
URL: https://github.com/apache/maven-shade-plugin/pull/175#issuecomment-1440224606

   > Very nice. Can't believe we still haven't upgraded to 3.2.5.
   
   btw, this one catches nicely all these :wink: https://issues.apache.org/jira/browse/MNG-7706


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org