You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "slawekjaranowski (via GitHub)" <gi...@apache.org> on 2023/01/24 12:23:00 UTC
[GitHub] [maven-jar-plugin] slawekjaranowski commented on a diff in pull request #57: [MJAR-292] - Detect MRJAR and add Multi-Release manifest entry
slawekjaranowski commented on code in PR #57:
URL: https://github.com/apache/maven-jar-plugin/pull/57#discussion_r1085228950
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -225,23 +237,24 @@ public File createArchive()
jarContentFileSet.setIncludes( Arrays.asList( getIncludes() ) );
jarContentFileSet.setExcludes( Arrays.asList( getExcludes() ) );
- boolean containsModuleDescriptor = false;
String[] includedFiles = fileSetManager.getIncludedFiles( jarContentFileSet );
- for ( String includedFile : includedFiles )
+
+ // May give false positives if the files is named as module descriptor
+ // but is not in the root of the archive or in the versioned area
+ // (and hence not actually a module descriptor).
+ // That is fine since the modular Jar archiver will gracefully
+ // handle such case.
+ // And also such case is unlikely to happen as file ending
+ // with "module-info.class" is unlikely to be included in Jar file
+ // unless it is a module descriptor.
+ boolean containsModuleDescriptor =
+ Arrays.stream( includedFiles ).anyMatch( p -> p.endsWith( MODULE_DESCRIPTOR_FILE_NAME ) );
Review Comment:
I would like to move this lines to pleace where is used about lines 260
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -225,23 +237,24 @@ public File createArchive()
jarContentFileSet.setIncludes( Arrays.asList( getIncludes() ) );
jarContentFileSet.setExcludes( Arrays.asList( getExcludes() ) );
- boolean containsModuleDescriptor = false;
String[] includedFiles = fileSetManager.getIncludedFiles( jarContentFileSet );
- for ( String includedFile : includedFiles )
+
+ // May give false positives if the files is named as module descriptor
+ // but is not in the root of the archive or in the versioned area
+ // (and hence not actually a module descriptor).
+ // That is fine since the modular Jar archiver will gracefully
+ // handle such case.
+ // And also such case is unlikely to happen as file ending
+ // with "module-info.class" is unlikely to be included in Jar file
+ // unless it is a module descriptor.
+ boolean containsModuleDescriptor =
+ Arrays.stream( includedFiles ).anyMatch( p -> p.endsWith( MODULE_DESCRIPTOR_FILE_NAME ) );
+
+ if ( detectMultiReleaseJar && Arrays.stream( includedFiles ).anyMatch( p -> p.startsWith( "META-INF" + SEPARATOR
+ + "versions" + SEPARATOR ) ) )
{
- // May give false positives if the files is named as module descriptor
- // but is not in the root of the archive or in the versioned area
- // (and hence not actually a module descriptor).
- // That is fine since the modular Jar archiver will gracefully
- // handle such case.
- // And also such case is unlikely to happen as file ending
- // with "module-info.class" is unlikely to be included in Jar file
- // unless it is a module descriptor.
- if ( includedFile.endsWith( MODULE_DESCRIPTOR_FILE_NAME ) )
- {
- containsModuleDescriptor = true;
- break;
- }
+ getLog().debug( "Adding 'Multi-Release: true' manifest entry." );
+ archive.addManifestEntry( "Multi-Release", "true" );
}
String archiverName = containsModuleDescriptor ? "mjar" : "jar";
Review Comment:
Should we also use `mjar` as archiver name when detect in steps above that is Multi-Release?
I don't know what is difference between `mjar` and `jar` archiver ...
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -225,23 +237,24 @@ public File createArchive()
jarContentFileSet.setIncludes( Arrays.asList( getIncludes() ) );
jarContentFileSet.setExcludes( Arrays.asList( getExcludes() ) );
- boolean containsModuleDescriptor = false;
String[] includedFiles = fileSetManager.getIncludedFiles( jarContentFileSet );
- for ( String includedFile : includedFiles )
+
+ // May give false positives if the files is named as module descriptor
+ // but is not in the root of the archive or in the versioned area
+ // (and hence not actually a module descriptor).
+ // That is fine since the modular Jar archiver will gracefully
+ // handle such case.
+ // And also such case is unlikely to happen as file ending
+ // with "module-info.class" is unlikely to be included in Jar file
+ // unless it is a module descriptor.
+ boolean containsModuleDescriptor =
+ Arrays.stream( includedFiles ).anyMatch( p -> p.endsWith( MODULE_DESCRIPTOR_FILE_NAME ) );
+
+ if ( detectMultiReleaseJar && Arrays.stream( includedFiles ).anyMatch( p -> p.startsWith( "META-INF" + SEPARATOR
+ + "versions" + SEPARATOR ) ) )
{
- // May give false positives if the files is named as module descriptor
- // but is not in the root of the archive or in the versioned area
- // (and hence not actually a module descriptor).
- // That is fine since the modular Jar archiver will gracefully
- // handle such case.
- // And also such case is unlikely to happen as file ending
- // with "module-info.class" is unlikely to be included in Jar file
- // unless it is a module descriptor.
- if ( includedFile.endsWith( MODULE_DESCRIPTOR_FILE_NAME ) )
- {
- containsModuleDescriptor = true;
- break;
- }
+ getLog().debug( "Adding 'Multi-Release: true' manifest entry." );
+ archive.addManifestEntry( "Multi-Release", "true" );
Review Comment:
What when user already defined such configuration ... do we need to discover in such case?
--
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