[GitHub] [maven-shade-plugin] kriegaex commented on a diff in pull request #110: [MSHADE-400] Self-minimisation with custom entry points

2023-03-05 Thread via GitHub


kriegaex commented on code in PR #110:
URL: 
https://github.com/apache/maven-shade-plugin/pull/110#discussion_r1125638933


##
src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java:
##
@@ -964,11 +992,16 @@ private List getFilters()
 
 if ( minimizeJar )
 {
-getLog().info( "Minimizing jar " + project.getArtifact() );
+if ( entryPoints == null )
+{
+entryPoints = new HashSet<>();
+}
+getLog().info( "Minimizing jar " + project.getArtifact()
++ ( entryPoints.isEmpty() ? "" : ", entry points = " + 
entryPoints ) );

Review Comment:
   Which I did in the new commit already.



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven-shade-plugin] kriegaex commented on a diff in pull request #110: [MSHADE-400] Self-minimisation with custom entry points

2023-03-05 Thread via GitHub


kriegaex commented on code in PR #110:
URL: 
https://github.com/apache/maven-shade-plugin/pull/110#discussion_r1125638854


##
src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java:
##
@@ -156,6 +204,43 @@ private void removeServices( final MavenProject project, 
final Clazzpath cp )
 while ( repeatScan );
 }
 
+private boolean removeServicesFromDir( Clazzpath cp, Set 
neededClasses, String fileName )
+{
+final File servicesDir = new File( fileName, "META-INF/services/" );
+if ( !servicesDir.isDirectory() )
+{
+return false;
+}
+final File[] serviceProviderConfigFiles = servicesDir.listFiles();
+if ( serviceProviderConfigFiles == null || 
serviceProviderConfigFiles.length == 0 )
+{
+return false;
+}
+
+boolean repeatScan = false;
+for ( File serviceProviderConfigFile : serviceProviderConfigFiles )
+{
+final String serviceClassName = 
serviceProviderConfigFile.getName();
+final boolean isNeededClass = neededClasses.contains( cp.getClazz( 
serviceClassName ) );
+if ( !isNeededClass )
+{
+continue;
+}
+
+try ( final BufferedReader configFileReader = new BufferedReader(
+new InputStreamReader( new FileInputStream( 
serviceProviderConfigFile ), UTF_8 ) ) )
+{
+// check whether the found classes use services in turn
+repeatScan |= scanServiceProviderConfigFile( cp, 
configFileReader );
+}
+catch ( final IOException e )
+{
+log.warn( e.getMessage() );

Review Comment:
   Actually, it was easy enough to comply with your wish (see the new commit). 
Secondly, Guillaume's authorship of the code comes from his squashing one of my 
previous PRs, so actually it is fair enough for me to improve both places.



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven-shade-plugin] kriegaex commented on a diff in pull request #110: [MSHADE-400] Self-minimisation with custom entry points

2023-03-05 Thread via GitHub


kriegaex commented on code in PR #110:
URL: 
https://github.com/apache/maven-shade-plugin/pull/110#discussion_r1125616374


##
src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java:
##
@@ -964,11 +992,16 @@ private List getFilters()
 
 if ( minimizeJar )
 {
-getLog().info( "Minimizing jar " + project.getArtifact() );
+if ( entryPoints == null )
+{
+entryPoints = new HashSet<>();
+}
+getLog().info( "Minimizing jar " + project.getArtifact()
++ ( entryPoints.isEmpty() ? "" : ", entry points = " + 
entryPoints ) );

Review Comment:
   So I should not log any info at all about the minimisation step, or just not 
the entry points? I always like to see what is going on in a Maven build, and 
the information in debug mode is somewhat overwhelming. But if you tell me how 
you like it or how it is customary, I shall comply.



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven-shade-plugin] kriegaex commented on a diff in pull request #110: [MSHADE-400] Self-minimisation with custom entry points

2023-03-04 Thread via GitHub


kriegaex commented on code in PR #110:
URL: 
https://github.com/apache/maven-shade-plugin/pull/110#discussion_r1125615482


##
src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java:
##
@@ -156,6 +204,43 @@ private void removeServices( final MavenProject project, 
final Clazzpath cp )
 while ( repeatScan );
 }
 
+private boolean removeServicesFromDir( Clazzpath cp, Set 
neededClasses, String fileName )
+{
+final File servicesDir = new File( fileName, "META-INF/services/" );
+if ( !servicesDir.isDirectory() )
+{
+return false;
+}
+final File[] serviceProviderConfigFiles = servicesDir.listFiles();
+if ( serviceProviderConfigFiles == null || 
serviceProviderConfigFiles.length == 0 )
+{
+return false;
+}
+
+boolean repeatScan = false;
+for ( File serviceProviderConfigFile : serviceProviderConfigFiles )
+{
+final String serviceClassName = 
serviceProviderConfigFile.getName();
+final boolean isNeededClass = neededClasses.contains( cp.getClazz( 
serviceClassName ) );
+if ( !isNeededClass )
+{
+continue;
+}
+
+try ( final BufferedReader configFileReader = new BufferedReader(
+new InputStreamReader( new FileInputStream( 
serviceProviderConfigFile ), UTF_8 ) ) )
+{
+// check whether the found classes use services in turn
+repeatScan |= scanServiceProviderConfigFile( cp, 
configFileReader );
+}
+catch ( final IOException e )
+{
+log.warn( e.getMessage() );

Review Comment:
   I did it the same way @gnodet committed it a few lines down in 
`removeServicesFromJar`. If one gets a decent message, then the other should, 
too. What would you have me do?
   
   Besides, I am not a Maven plugin developer, normally I just use them. When 
referring to "verbose mode", do you simply mean to log the exception on debug 
level?



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven-shade-plugin] kriegaex commented on a diff in pull request #110: [MSHADE-400] Self-minimisation with custom entry points

2023-03-04 Thread via GitHub


kriegaex commented on code in PR #110:
URL: 
https://github.com/apache/maven-shade-plugin/pull/110#discussion_r1125615482


##
src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java:
##
@@ -156,6 +204,43 @@ private void removeServices( final MavenProject project, 
final Clazzpath cp )
 while ( repeatScan );
 }
 
+private boolean removeServicesFromDir( Clazzpath cp, Set 
neededClasses, String fileName )
+{
+final File servicesDir = new File( fileName, "META-INF/services/" );
+if ( !servicesDir.isDirectory() )
+{
+return false;
+}
+final File[] serviceProviderConfigFiles = servicesDir.listFiles();
+if ( serviceProviderConfigFiles == null || 
serviceProviderConfigFiles.length == 0 )
+{
+return false;
+}
+
+boolean repeatScan = false;
+for ( File serviceProviderConfigFile : serviceProviderConfigFiles )
+{
+final String serviceClassName = 
serviceProviderConfigFile.getName();
+final boolean isNeededClass = neededClasses.contains( cp.getClazz( 
serviceClassName ) );
+if ( !isNeededClass )
+{
+continue;
+}
+
+try ( final BufferedReader configFileReader = new BufferedReader(
+new InputStreamReader( new FileInputStream( 
serviceProviderConfigFile ), UTF_8 ) ) )
+{
+// check whether the found classes use services in turn
+repeatScan |= scanServiceProviderConfigFile( cp, 
configFileReader );
+}
+catch ( final IOException e )
+{
+log.warn( e.getMessage() );

Review Comment:
   I did it the same way @gnodet committed it a few lines down in 
`removeServicesFromJar`. If one gets a decent message, then the other should, 
too. What would you have me do?



-- 
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-unsubscr...@maven.apache.org

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