rmannibucau commented on code in PR #508:
URL: https://github.com/apache/maven-jar-plugin/pull/508#discussion_r3777392170
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -207,167 +205,199 @@ protected final Log getLog() {
protected abstract String getType();
/**
- * Returns the JAR file to generate, based on an optional classifier.
+ * {@return the scope of dependencies}
+ * It should be {@link PathScope#MAIN_COMPILE} or {@link
PathScope#TEST_COMPILE}.
+ * Note that we use compile scope rather than runtime scope because
dependencies
+ * cannot appear in {@code requires} statement if they didn't had compile
scope.
+ */
+ protected abstract PathScope getDependencyScope();
+
+ /**
+ * {@return the JAR tool to use for archiving the code}
*
- * @param basedir the output directory
- * @param resultFinalName the name of the JAR file
- * @param classifier an optional classifier
- * @return the file to generate
+ * @throws MojoException if no JAR tool was found
+ *
+ * @since 4.0.0-beta-2
*/
- protected Path getJarFile(Path basedir, String resultFinalName, String
classifier) {
- Objects.requireNonNull(basedir, "basedir is not allowed to be null");
- Objects.requireNonNull(resultFinalName, "finalName is not allowed to
be null");
- String fileName = resultFinalName + (hasClassifier(classifier) ? '-' +
classifier : "") + ".jar";
- return basedir.resolve(fileName);
+ protected ToolProvider getJarTool() throws MojoException {
+ return ToolProvider.findFirst(toolId).orElseThrow(() -> new
MojoException("No such \"" + toolId + "\" tool."));
}
/**
- * Generates the JAR.
+ * Returns whether the specified Java version is supported.
*
- * @return the path to the created archive file
- * @throws MojoException in case of an error
+ * @param release name of an {@link SourceVersion} enumeration constant
+ * @return whether the current environment support that version
*/
- public Path createArchive() throws MojoException {
- Path basedir = outputDirectory != null
- ? outputDirectory
- : Path.of(project.getBuild().getDirectory());
- String resultFinalName =
- finalName != null ? finalName :
project.getBuild().getFinalName();
- Path jarFile = getJarFile(basedir, resultFinalName, getClassifier());
-
- FileSetManager fileSetManager = new FileSetManager();
- FileSet jarContentFileSet = new FileSet();
-
jarContentFileSet.setDirectory(getClassesDirectory().toAbsolutePath().toString());
- jarContentFileSet.setIncludes(Arrays.asList(getIncludes()));
- jarContentFileSet.setExcludes(Arrays.asList(getExcludes()));
-
- String[] includedFiles =
fileSetManager.getIncludedFiles(jarContentFileSet);
-
- if (detectMultiReleaseJar
- && Arrays.stream(includedFiles)
- .anyMatch(
- p -> p.startsWith("META-INF" +
File.separatorChar + "versions" + File.separatorChar))) {
- getLog().debug("Adding 'Multi-Release: true' manifest entry.");
- archive.addManifestEntry(Attributes.Name.MULTI_RELEASE.toString(),
"true");
+ private static boolean isSupported(String release) {
Review Comment:
> We will need an API from the tool provider API telling us what is the
version of the tool.
I don't think it will happen since tool provider is generic enough to not
have much compiler knowledge in its own signature, tool provider but the java
compiler could maybe
(https://docs.oracle.com/javase/8/docs/api/javax/tools/Tool.html#getSourceVersions--).
> Therefore, it is not a regression if the tool provider support is
incomplete.
Well, it is a bug compared to previous experience if you do use the new
feature so even if we might not consider it blocking it is bothering to say we
rewrote it in a way which doesn't work.
One interesting excercise is to ensure the supported compilers are still
supported (thinking to ECJ for ex which is known faster than javac).
Agree forking can come later to avoid leaks in mvnd or alike.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
