This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push:
new dec90acf24 [MNG-7866] Improvements to the logging API usage (technical
debt) (#1220)
dec90acf24 is described below
commit dec90acf24f9188b946e89949b488e3f586f0be0
Author: sebastien-doyon <sebastie...@gmail.com>
AuthorDate: Fri Sep 22 05:07:17 2023 -0400
[MNG-7866] Improvements to the logging API usage (technical debt) (#1220)
---
.../project/artifact/MavenMetadataSource.java | 16 +-
.../maven/toolchain/DefaultToolchainsBuilder.java | 2 +-
maven-core/pom.xml | 2 +-
.../main/java/org/apache/maven/DefaultMaven.java | 18 ++-
.../maven/classrealm/DefaultClassRealmManager.java | 66 +++------
.../internal/aether/LoggingRepositoryListener.java | 39 +++--
.../org/apache/maven/internal/impl/DefaultLog.java | 32 +++-
.../apache/maven/lifecycle/DefaultLifecycles.java | 13 +-
.../internal/DefaultLifecyclePluginAnalyzer.java | 6 +-
.../internal/LifecycleDependencyResolver.java | 2 +-
.../multithreaded/MultiThreadedBuilder.java | 4 +-
.../plugin/internal/DefaultMavenPluginManager.java | 16 +-
.../DefaultPluginDependenciesResolver.java | 17 ++-
.../maven/plugin/internal/MojoLogWrapper.java | 32 +++-
.../internal/DefaultPluginPrefixResolver.java | 30 ++--
.../internal/DefaultPluginVersionResolver.java | 34 +++--
.../collector/MultiModuleCollectionStrategy.java | 5 +-
.../apache/maven/toolchain/DefaultToolchain.java | 4 +-
.../classrealm/DefaultClassRealmManagerTest.java | 164 +++++++++++++++++++++
.../maven/toolchain/DefaultToolchainTest.java | 4 +-
20 files changed, 352 insertions(+), 154 deletions(-)
diff --git
a/maven-compat/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java
b/maven-compat/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java
index 4e0877e9eb..4240cecba5 100644
---
a/maven-compat/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java
+++
b/maven-compat/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java
@@ -633,11 +633,19 @@ public class MavenMetadataSource implements
ArtifactMetadataSource {
if (artifact.getDependencyTrail() != null
&& artifact.getDependencyTrail().size() == 1) {
- logger.warn("While downloading " +
pomArtifact.getGroupId() + ":"
- + pomArtifact.getArtifactId() + ":" +
pomArtifact.getVersion() + message);
+ logger.warn(
+ "While downloading {}:{}:{}{}",
+ pomArtifact.getGroupId(),
+ pomArtifact.getArtifactId(),
+ pomArtifact.getVersion(),
+ message);
} else {
- logger.debug("While downloading " +
pomArtifact.getGroupId() + ":"
- + pomArtifact.getArtifactId() + ":" +
pomArtifact.getVersion() + message);
+ logger.debug(
+ "While downloading {}:{}:{}{}",
+ pomArtifact.getGroupId(),
+ pomArtifact.getArtifactId(),
+ pomArtifact.getVersion(),
+ message);
}
} else {
done = true;
diff --git
a/maven-compat/src/main/java/org/apache/maven/toolchain/DefaultToolchainsBuilder.java
b/maven-compat/src/main/java/org/apache/maven/toolchain/DefaultToolchainsBuilder.java
index c2c608b951..e78fd2ca8a 100644
---
a/maven-compat/src/main/java/org/apache/maven/toolchain/DefaultToolchainsBuilder.java
+++
b/maven-compat/src/main/java/org/apache/maven/toolchain/DefaultToolchainsBuilder.java
@@ -51,7 +51,7 @@ public class DefaultToolchainsBuilder implements
ToolchainsBuilder {
}
} else if (userToolchainsFile != null) {
- logger.debug("Toolchains configuration was not found at " +
userToolchainsFile);
+ logger.debug("Toolchains configuration was not found at {}",
userToolchainsFile);
}
return toolchains;
diff --git a/maven-core/pom.xml b/maven-core/pom.xml
index 87484b5a6b..6b20529582 100644
--- a/maven-core/pom.xml
+++ b/maven-core/pom.xml
@@ -151,7 +151,7 @@ under the License.
</dependency>
<dependency>
<groupId>org.mockito</groupId>
- <artifactId>mockito-core</artifactId>
+ <artifactId>mockito-inline</artifactId>
<scope>test</scope>
</dependency>
<dependency>
diff --git a/maven-core/src/main/java/org/apache/maven/DefaultMaven.java
b/maven-core/src/main/java/org/apache/maven/DefaultMaven.java
index 18f0dea4d3..f49d5409a4 100644
--- a/maven-core/src/main/java/org/apache/maven/DefaultMaven.java
+++ b/maven-core/src/main/java/org/apache/maven/DefaultMaven.java
@@ -410,7 +410,7 @@ public class DefaultMaven implements Maven {
private void validateLocalRepository(MavenExecutionRequest request) throws
IOException {
File localRepoDir = request.getLocalRepositoryPath();
- logger.debug("Using local repository at " + localRepoDir);
+ logger.debug("Using local repository at {}", localRepoDir);
localRepoDir.mkdirs();
@@ -426,7 +426,7 @@ public class DefaultMaven implements Maven {
foundComponents.addAll(container.lookupList(role));
} catch (ComponentLookupException e) {
// this is just silly, lookupList should return an empty list!
- logger.warn("Failed to lookup " + role + ": " + e.getMessage());
+ logger.warn("Failed to lookup {}: {}", role, e.getMessage());
}
foundComponents.addAll(getProjectScopedExtensionComponents(projects,
role));
@@ -455,7 +455,7 @@ public class DefaultMaven implements Maven {
foundComponents.addAll(container.lookupList(role));
} catch (ComponentLookupException e) {
// this is just silly, lookupList should return an
empty list!
- logger.warn("Failed to lookup " + role + ": " +
e.getMessage());
+ logger.warn("Failed to lookup {}: {}", role,
e.getMessage());
}
}
}
@@ -479,11 +479,13 @@ public class DefaultMaven implements Maven {
Prerequisites prerequisites =
mavenProject.getModel().getDelegate().getPrerequisites();
if (prerequisites != null && prerequisites.getMaven() != null)
{
- logger.warn("The project " + mavenProject.getId() + " uses
prerequisites"
- + " which is only intended for maven-plugin
projects "
- + "but not for non maven-plugin projects. "
- + "For such purposes you should use the
maven-enforcer-plugin. "
- + "See
https://maven.apache.org/enforcer/enforcer-rules/requireMavenVersion.html";);
+ logger.warn(
+ "The project {} uses prerequisites"
+ + " which is only intended for
maven-plugin projects "
+ + "but not for non maven-plugin projects. "
+ + "For such purposes you should use the
maven-enforcer-plugin. "
+ + "See
https://maven.apache.org/enforcer/enforcer-rules/requireMavenVersion.html";,
+ mavenProject.getId());
}
}
}
diff --git
a/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java
b/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java
index 06b412e2fa..512e7cae8f 100644
---
a/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java
+++
b/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java
@@ -26,7 +26,6 @@ import java.io.File;
import java.net.MalformedURLException;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -116,9 +115,7 @@ public class DefaultClassRealmManager implements
ClassRealmManager {
try {
ClassRealm classRealm = world.newRealm(realmId, null);
- if (logger.isDebugEnabled()) {
- logger.debug("Created new class realm " + realmId);
- }
+ logger.debug("Created new class realm {}", realmId);
return classRealm;
} catch (DuplicateRealmException e) {
@@ -151,17 +148,14 @@ public class DefaultClassRealmManager implements
ClassRealmManager {
List<String> parentImports,
Map<String, ClassLoader> foreignImports,
List<Artifact> artifacts) {
- Set<String> artifactIds = new LinkedHashSet<>();
+ List<ClassRealmConstituent> constituents = new ArrayList<>(artifacts
== null ? 0 : artifacts.size());
- List<ClassRealmConstituent> constituents = new ArrayList<>();
-
- if (artifacts != null) {
+ if (artifacts != null && !artifacts.isEmpty()) {
for (Artifact artifact : artifacts) {
- if (!isProvidedArtifact(artifact)) {
- artifactIds.add(getId(artifact));
- if (artifact.getFile() != null) {
- constituents.add(new
ArtifactClassRealmConstituent(artifact));
- }
+ if (!isProvidedArtifact(artifact) && artifact.getFile() !=
null) {
+ constituents.add(new
ArtifactClassRealmConstituent(artifact));
+ } else if (logger.isDebugEnabled()) {
+ logger.debug(" Excluded: {}", getId(artifact));
}
}
}
@@ -188,15 +182,7 @@ public class DefaultClassRealmManager implements
ClassRealmManager {
wireRealm(classRealm, parentImports, foreignImports);
- Set<String> includedIds = populateRealm(classRealm, constituents);
-
- if (logger.isDebugEnabled()) {
- artifactIds.removeAll(includedIds);
-
- for (String id : artifactIds) {
- logger.debug(" Excluded: " + id);
- }
- }
+ populateRealm(classRealm, constituents);
return classRealm;
}
@@ -299,21 +285,15 @@ public class DefaultClassRealmManager implements
ClassRealmManager {
}
}
- private Set<String> populateRealm(ClassRealm classRealm,
List<ClassRealmConstituent> constituents) {
- Set<String> includedIds = new LinkedHashSet<>();
-
- if (logger.isDebugEnabled()) {
- logger.debug("Populating class realm " + classRealm.getId());
- }
+ private void populateRealm(ClassRealm classRealm,
List<ClassRealmConstituent> constituents) {
+ logger.debug("Populating class realm {}", classRealm.getId());
for (ClassRealmConstituent constituent : constituents) {
File file = constituent.getFile();
- String id = getId(constituent);
- includedIds.add(id);
-
if (logger.isDebugEnabled()) {
- logger.debug(" Included: " + id);
+ String id = getId(constituent);
+ logger.debug(" Included: {}", id);
}
try {
@@ -323,47 +303,37 @@ public class DefaultClassRealmManager implements
ClassRealmManager {
logger.error(e.getMessage(), e);
}
}
-
- return includedIds;
}
private void wireRealm(ClassRealm classRealm, List<String> parentImports,
Map<String, ClassLoader> foreignImports) {
if (foreignImports != null && !foreignImports.isEmpty()) {
- if (logger.isDebugEnabled()) {
- logger.debug("Importing foreign packages into class realm " +
classRealm.getId());
- }
+ logger.debug("Importing foreign packages into class realm {}",
classRealm.getId());
for (Map.Entry<String, ClassLoader> entry :
foreignImports.entrySet()) {
ClassLoader importedRealm = entry.getValue();
String imp = entry.getKey();
- if (logger.isDebugEnabled()) {
- logger.debug(" Imported: " + imp + " < " +
getId(importedRealm));
- }
+ logger.debug(" Imported: {} < {}", imp, getId(importedRealm));
classRealm.importFrom(importedRealm, imp);
}
}
if (parentImports != null && !parentImports.isEmpty()) {
- if (logger.isDebugEnabled()) {
- logger.debug("Importing parent packages into class realm " +
classRealm.getId());
- }
+ logger.debug("Importing parent packages into class realm {}",
classRealm.getId());
for (String imp : parentImports) {
- if (logger.isDebugEnabled()) {
- logger.debug(" Imported: " + imp + " < " +
getId(classRealm.getParentClassLoader()));
- }
+ logger.debug(" Imported: {} < {}", imp,
getId(classRealm.getParentClassLoader()));
classRealm.importFromParent(imp);
}
}
}
- private String getId(ClassLoader classLoader) {
+ private static Object getId(ClassLoader classLoader) {
if (classLoader instanceof ClassRealm) {
return ((ClassRealm) classLoader).getId();
}
- return String.valueOf(classLoader);
+ return classLoader;
}
}
diff --git
a/maven-core/src/main/java/org/apache/maven/internal/aether/LoggingRepositoryListener.java
b/maven-core/src/main/java/org/apache/maven/internal/aether/LoggingRepositoryListener.java
index d752da6a8d..fe9c074b33 100644
---
a/maven-core/src/main/java/org/apache/maven/internal/aether/LoggingRepositoryListener.java
+++
b/maven-core/src/main/java/org/apache/maven/internal/aether/LoggingRepositoryListener.java
@@ -37,12 +37,12 @@ class LoggingRepositoryListener extends
AbstractRepositoryListener {
@Override
public void artifactInstalling(RepositoryEvent event) {
- logger.info("Installing " + event.getArtifact().getFile() + " to " +
event.getFile());
+ logger.info("Installing {} to {}", event.getArtifact().getFile(),
event.getFile());
}
@Override
public void metadataInstalling(RepositoryEvent event) {
- logger.debug("Installing " + event.getMetadata() + " to " +
event.getFile());
+ logger.debug("Installing {} to {}", event.getMetadata(),
event.getFile());
}
@Override
@@ -63,48 +63,45 @@ class LoggingRepositoryListener extends
AbstractRepositoryListener {
public void metadataInvalid(RepositoryEvent event) {
Exception exception = event.getException();
- StringBuilder buffer = new StringBuilder(256);
- buffer.append("The metadata ");
+ Object metadata;
if (event.getMetadata().getFile() != null) {
- buffer.append(event.getMetadata().getFile());
+ metadata = event.getMetadata().getFile();
} else {
- buffer.append(event.getMetadata());
+ metadata = event.getMetadata();
}
+ String errorType = " is invalid";
if (exception instanceof FileNotFoundException) {
- buffer.append(" is inaccessible");
- } else {
- buffer.append(" is invalid");
+ errorType = " is inaccessible";
}
+ String msg = "";
if (exception != null) {
- buffer.append(": ");
- buffer.append(exception.getMessage());
+ msg = ": " + exception.getMessage();
}
if (logger.isDebugEnabled()) {
- logger.warn(buffer.toString(), exception);
+ logger.warn("The metadata {} {}{}", metadata, errorType, msg,
exception);
} else {
- logger.warn(buffer.toString());
+ logger.warn("The metadata {} {}{}", metadata, errorType, msg);
}
}
@Override
public void artifactDescriptorInvalid(RepositoryEvent event) {
- StringBuilder buffer = new StringBuilder(256);
- buffer.append("The POM for ");
- buffer.append(event.getArtifact());
- buffer.append(" is invalid, transitive dependencies (if any) will not
be available");
-
+ // The exception stack trace is not really interesting here
+ // but the message itself may be quite details and span multiple
+ // lines with errors in it, so only display it at debug level.
+ String msg = "The POM for {} is invalid, transitive dependencies (if
any) will not be available: {}";
if (logger.isDebugEnabled()) {
- logger.warn(buffer + ": " + event.getException().getMessage());
+ logger.warn(msg, event.getArtifact(),
event.getException().getMessage());
} else {
- logger.warn(buffer + ", enable verbose output (-X) for more
details");
+ logger.warn(msg, event.getArtifact(), "enable verbose output (-X)
for more details");
}
}
@Override
public void artifactDescriptorMissing(RepositoryEvent event) {
- logger.warn("The POM for " + event.getArtifact() + " is missing, no
dependency information available");
+ logger.warn("The POM for {} is missing, no dependency information
available", event.getArtifact());
}
}
diff --git
a/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLog.java
b/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLog.java
index c1a2d8d2e3..fa26a10083 100644
--- a/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLog.java
+++ b/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLog.java
@@ -33,12 +33,16 @@ public class DefaultLog implements Log {
}
public void debug(CharSequence content) {
- logger.debug(toString(content));
+ if (isDebugEnabled()) {
+ logger.debug(toString(content));
+ }
}
@Override
public void debug(CharSequence content, Throwable error) {
- logger.debug(toString(content), error);
+ if (isDebugEnabled()) {
+ logger.debug(toString(content), error);
+ }
}
@Override
@@ -62,12 +66,16 @@ public class DefaultLog implements Log {
@Override
public void info(CharSequence content) {
- logger.info(toString(content));
+ if (isInfoEnabled()) {
+ logger.info(toString(content));
+ }
}
@Override
public void info(CharSequence content, Throwable error) {
- logger.info(toString(content), error);
+ if (isInfoEnabled()) {
+ logger.info(toString(content), error);
+ }
}
@Override
@@ -91,12 +99,16 @@ public class DefaultLog implements Log {
@Override
public void warn(CharSequence content) {
- logger.warn(toString(content));
+ if (isWarnEnabled()) {
+ logger.warn(toString(content));
+ }
}
@Override
public void warn(CharSequence content, Throwable error) {
- logger.warn(toString(content), error);
+ if (isWarnEnabled()) {
+ logger.warn(toString(content), error);
+ }
}
@Override
@@ -120,12 +132,16 @@ public class DefaultLog implements Log {
@Override
public void error(CharSequence content) {
- logger.error(toString(content));
+ if (isErrorEnabled()) {
+ logger.error(toString(content));
+ }
}
@Override
public void error(CharSequence content, Throwable error) {
- logger.error(toString(content), error);
+ if (isErrorEnabled()) {
+ logger.error(toString(content), error);
+ }
}
@Override
diff --git
a/maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java
b/maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java
index 2a6868baf5..76cb5a0e5a 100644
--- a/maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java
+++ b/maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java
@@ -94,18 +94,19 @@ public class DefaultLifecycles {
Map<String, Lifecycle> phaseToLifecycleMap = new HashMap<>();
for (Lifecycle lifecycle : getLifeCycles()) {
- if (logger.isDebugEnabled()) {
- logger.debug("Lifecycle " + lifecycle);
- }
+ logger.debug("Lifecycle {}", lifecycle);
for (String phase : lifecycle.getPhases()) {
// The first definition wins.
if (!phaseToLifecycleMap.containsKey(phase)) {
phaseToLifecycleMap.put(phase, lifecycle);
- } else {
+ } else if (logger.isWarnEnabled()) {
Lifecycle original = phaseToLifecycleMap.get(phase);
- logger.warn("Duplicated lifecycle phase " + phase + ".
Defined in " + original.getId()
- + " but also in " + lifecycle.getId());
+ logger.warn(
+ "Duplicated lifecycle phase {}. Defined in {} but
also in {}",
+ phase,
+ original.getId(),
+ lifecycle.getId());
}
}
}
diff --git
a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecyclePluginAnalyzer.java
b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecyclePluginAnalyzer.java
index b3c1dc29fb..652de2ca03 100644
---
a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecyclePluginAnalyzer.java
+++
b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecyclePluginAnalyzer.java
@@ -154,8 +154,10 @@ public class DefaultLifecyclePluginAnalyzer implements
LifeCyclePluginAnalyzer {
GoalSpec gs = parseGoalSpec(mojo.getGoal());
if (gs == null) {
- logger.warn("Ignored invalid goal specification '" +
mojo.getGoal()
- + "' from lifecycle mapping for phase " + phase);
+ logger.warn(
+ "Ignored invalid goal specification '{}' from
lifecycle mapping for phase {}",
+ mojo.getGoal(),
+ phase);
continue;
}
diff --git
a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java
b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java
index 7c30835ebb..bd5fc4626b 100644
---
a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java
+++
b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java
@@ -250,7 +250,7 @@ public class LifecycleDependencyResolver {
+ " but seem to be part of the reactor:");
for (Dependency dependency :
result.getUnresolvedDependencies()) {
- logger.warn("o " + dependency);
+ logger.warn("o {}", dependency);
}
logger.warn("Try running the build up to the lifecycle phase
\"package\"");
diff --git
a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/MultiThreadedBuilder.java
b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/MultiThreadedBuilder.java
index 607a883b85..2a76afb50d 100644
---
a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/MultiThreadedBuilder.java
+++
b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/MultiThreadedBuilder.java
@@ -138,7 +138,7 @@ public class MultiThreadedBuilder implements Builder {
// schedule independent projects
for (MavenProject mavenProject : analyzer.getRootSchedulableBuilds()) {
ProjectSegment projectSegment = projectBuildList.get(mavenProject);
- logger.debug("Scheduling: " + projectSegment.getProject());
+ logger.debug("Scheduling: {}", projectSegment.getProject());
Callable<ProjectSegment> cb = createBuildCallable(
rootSession, projectSegment, reactorContext, taskSegment,
muxer, duplicateArtifactIds);
service.submit(cb);
@@ -158,7 +158,7 @@ public class MultiThreadedBuilder implements Builder {
analyzer.markAsFinished(projectBuild.getProject());
for (MavenProject mavenProject : newItemsThatCanBeBuilt) {
ProjectSegment scheduledDependent =
projectBuildList.get(mavenProject);
- logger.debug("Scheduling: " + scheduledDependent);
+ logger.debug("Scheduling: {}", scheduledDependent);
Callable<ProjectSegment> cb = createBuildCallable(
rootSession,
scheduledDependent,
diff --git
a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java
b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java
index b4628d3183..ef1322d2fa 100644
---
a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java
+++
b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java
@@ -638,8 +638,10 @@ public class DefaultMavenPluginManager implements
MavenPluginManager {
ValidatingConfigurationListener validator =
new ValidatingConfigurationListener(mojo, mojoDescriptor,
listener);
- logger.debug("Configuring mojo execution '" +
mojoDescriptor.getId() + ':' + executionId + "' with "
- + configuratorId + " configurator -->");
+ if (logger.isDebugEnabled()) {
+ logger.debug("Configuring mojo execution '" +
mojoDescriptor.getId() + ':' + executionId + "' with "
+ + configuratorId + " configurator -->");
+ }
configurator.configureComponent(mojo, configuration,
expressionEvaluator, pluginRealm, validator);
@@ -749,10 +751,14 @@ public class DefaultMavenPluginManager implements
MavenPluginManager {
String goalExecId = mojoExecution.getGoal();
if (mojoExecution.getExecutionId() != null) {
- goalExecId += " {execution: " +
mojoExecution.getExecutionId() + "}";
+ logger.debug(
+ "Error releasing mojo for {} {execution: {}}",
+ goalExecId,
+ mojoExecution.getExecutionId(),
+ e);
+ } else {
+ logger.debug("Error releasing mojo for {}", goalExecId, e);
}
-
- logger.debug("Error releasing mojo for " + goalExecId, e);
}
}
}
diff --git
a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java
b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java
index d1f9b01111..2f7cb9af21 100644
---
a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java
+++
b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java
@@ -115,14 +115,15 @@ public class DefaultPluginDependenciesResolver implements
PluginDependenciesReso
pluginArtifact = result.getArtifact();
- if (logger.isWarnEnabled()) {
- if (!result.getRelocations().isEmpty()) {
- String message = pluginArtifact instanceof
org.apache.maven.repository.internal.RelocatedArtifact
- ?
((org.apache.maven.repository.internal.RelocatedArtifact)
pluginArtifact).getMessage()
- : null;
- logger.warn("The artifact " +
result.getRelocations().get(0) + " has been relocated to "
- + pluginArtifact + (message != null ? ": " +
message : ""));
- }
+ if (logger.isWarnEnabled() && !result.getRelocations().isEmpty()) {
+ String message = pluginArtifact instanceof
org.apache.maven.repository.internal.RelocatedArtifact
+ ? ": " +
((org.apache.maven.repository.internal.RelocatedArtifact)
pluginArtifact).getMessage()
+ : "";
+ logger.warn(
+ "The artifact {} has been relocated to {}{}",
+ result.getRelocations().get(0),
+ pluginArtifact,
+ message);
}
String requiredMavenVersion = (String)
result.getProperties().get("prerequisites.maven");
diff --git
a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogWrapper.java
b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogWrapper.java
index 06d10193d2..2ffd05ba85 100644
---
a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogWrapper.java
+++
b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogWrapper.java
@@ -33,7 +33,9 @@ public class MojoLogWrapper implements Log {
}
public void debug(CharSequence content) {
- logger.debug(toString(content));
+ if (isDebugEnabled()) {
+ logger.debug(toString(content));
+ }
}
private String toString(CharSequence content) {
@@ -46,7 +48,9 @@ public class MojoLogWrapper implements Log {
@Override
public void debug(CharSequence content, Throwable error) {
- logger.debug(toString(content), error);
+ if (isDebugEnabled()) {
+ logger.debug(toString(content), error);
+ }
}
@Override
@@ -56,12 +60,16 @@ public class MojoLogWrapper implements Log {
@Override
public void info(CharSequence content) {
- logger.info(toString(content));
+ if (isInfoEnabled()) {
+ logger.info(toString(content));
+ }
}
@Override
public void info(CharSequence content, Throwable error) {
- logger.info(toString(content), error);
+ if (isInfoEnabled()) {
+ logger.info(toString(content), error);
+ }
}
@Override
@@ -71,12 +79,16 @@ public class MojoLogWrapper implements Log {
@Override
public void warn(CharSequence content) {
- logger.warn(toString(content));
+ if (isWarnEnabled()) {
+ logger.warn(toString(content));
+ }
}
@Override
public void warn(CharSequence content, Throwable error) {
- logger.warn(toString(content), error);
+ if (isWarnEnabled()) {
+ logger.warn(toString(content), error);
+ }
}
@Override
@@ -86,12 +98,16 @@ public class MojoLogWrapper implements Log {
@Override
public void error(CharSequence content) {
- logger.error(toString(content));
+ if (isErrorEnabled()) {
+ logger.error(toString(content));
+ }
}
@Override
public void error(CharSequence content, Throwable error) {
- logger.error(toString(content), error);
+ if (isErrorEnabled()) {
+ logger.error(toString(content), error);
+ }
}
@Override
diff --git
a/maven-core/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java
b/maven-core/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java
index d99ff05aef..8bcea49685 100644
---
a/maven-core/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java
+++
b/maven-core/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java
@@ -78,7 +78,7 @@ public class DefaultPluginPrefixResolver implements
PluginPrefixResolver {
}
public PluginPrefixResult resolve(PluginPrefixRequest request) throws
NoPluginFoundForPrefixException {
- logger.debug("Resolving plugin prefix " + request.getPrefix() + " from
" + request.getPluginGroups());
+ logger.debug("Resolving plugin prefix {} from {}",
request.getPrefix(), request.getPluginGroups());
PluginPrefixResult result = resolveFromProject(request);
@@ -91,16 +91,21 @@ public class DefaultPluginPrefixResolver implements
PluginPrefixResolver {
request.getPluginGroups(),
request.getRepositorySession().getLocalRepository(),
request.getRepositories());
- } else if (logger.isDebugEnabled()) {
- logger.debug("Resolved plugin prefix " + request.getPrefix() +
" to " + result.getGroupId() + ":"
- + result.getArtifactId() + " from repository "
- + (result.getRepository() != null
- ? result.getRepository().getId()
- : "null"));
+ } else {
+ logger.debug(
+ "Resolved plugin prefix {} to {}:{} from repository
{}",
+ request.getPrefix(),
+ result.getGroupId(),
+ result.getArtifactId(),
+ (result.getRepository() != null ?
result.getRepository().getId() : "null"));
}
- } else if (logger.isDebugEnabled()) {
- logger.debug("Resolved plugin prefix " + request.getPrefix() + "
to " + result.getGroupId() + ":"
- + result.getArtifactId() + " from POM " +
request.getPom());
+ } else {
+ logger.debug(
+ "Resolved plugin prefix {} to {}:{} from POM {}",
+ request.getPrefix(),
+ result.getGroupId(),
+ result.getArtifactId(),
+ request.getPom());
}
return result;
@@ -133,10 +138,9 @@ public class DefaultPluginPrefixResolver implements
PluginPrefixResolver {
}
} catch (Exception e) {
if (logger.isDebugEnabled()) {
- logger.warn(
- "Failed to retrieve plugin descriptor for " +
plugin.getId() + ": " + e.getMessage(), e);
+ logger.warn("Failed to retrieve plugin descriptor for {}:
{}", plugin.getId(), e.getMessage(), e);
} else {
- logger.warn("Failed to retrieve plugin descriptor for " +
plugin.getId() + ": " + e.getMessage());
+ logger.warn("Failed to retrieve plugin descriptor for {}:
{}", plugin.getId(), e.getMessage());
}
}
}
diff --git
a/maven-core/src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java
b/maven-core/src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java
index b8c24cb86d..904606a2a0 100644
---
a/maven-core/src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java
+++
b/maven-core/src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java
@@ -105,19 +105,29 @@ public class DefaultPluginVersionResolver implements
PluginVersionResolver {
if (result == null) {
result = resolveFromRepository(request);
- if (logger.isDebugEnabled()) {
- logger.debug("Resolved plugin version for " +
request.getGroupId() + ":" + request.getArtifactId()
- + " to " + result.getVersion() + " from repository
" + result.getRepository());
- }
+ logger.debug(
+ "Resolved plugin version for {}:{} to {} from
repository {}",
+ request.getGroupId(),
+ request.getArtifactId(),
+ result.getVersion(),
+ result.getRepository());
cache.putIfAbsent(key, result);
- } else if (logger.isDebugEnabled()) {
- logger.debug("Reusing cached resolved plugin version for " +
request.getGroupId() + ":"
- + request.getArtifactId() + " to " +
result.getVersion() + " from POM " + request.getPom());
+ } else {
+ logger.debug(
+ "Reusing cached resolved plugin version for {}:{} to
{} from POM {}",
+ request.getGroupId(),
+ request.getArtifactId(),
+ result.getVersion(),
+ request.getPom());
}
- } else if (logger.isDebugEnabled()) {
- logger.debug("Resolved plugin version for " + request.getGroupId()
+ ":" + request.getArtifactId() + " to "
- + result.getVersion() + " from POM " + request.getPom());
+ } else {
+ logger.debug(
+ "Reusing cached resolved plugin version for {}:{} to {}
from POM {}",
+ request.getGroupId(),
+ request.getArtifactId(),
+ result.getVersion(),
+ request.getPom());
}
return result;
@@ -242,7 +252,7 @@ public class DefaultPluginVersionResolver implements
PluginVersionResolver {
pluginDescriptor = pluginManager.getPluginDescriptor(
plugin, request.getRepositories(),
request.getRepositorySession());
} catch (PluginResolutionException e) {
- logger.debug("Ignoring unresolvable plugin version " + version, e);
+ logger.debug("Ignoring unresolvable plugin version {}", version,
e);
return false;
} catch (Exception e) {
// ignore for now and delay failure to higher level processing
@@ -252,7 +262,7 @@ public class DefaultPluginVersionResolver implements
PluginVersionResolver {
try {
pluginManager.checkPrerequisites(pluginDescriptor);
} catch (Exception e) {
- logger.warn("Ignoring incompatible plugin version " + version, e);
+ logger.warn("Ignoring incompatible plugin version {}", version, e);
return false;
}
diff --git
a/maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java
b/maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java
index 9f81694d40..1216f081b2 100644
---
a/maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java
+++
b/maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java
@@ -107,8 +107,9 @@ public class MultiModuleCollectionStrategy implements
ProjectCollectionStrategy
"Maven detected that the requested POM file is part of
a multi-module project, "
+ "but could not find a pom.xml file in the
multi-module root directory '{}'.",
multiModuleProjectDirectory);
- LOGGER.info("The reactor is limited to all projects under: "
- + request.getPom().getParent());
+ LOGGER.info(
+ "The reactor is limited to all projects under: {}",
+ request.getPom().getParent());
return request.getPom();
}
diff --git
a/maven-core/src/main/java/org/apache/maven/toolchain/DefaultToolchain.java
b/maven-core/src/main/java/org/apache/maven/toolchain/DefaultToolchain.java
index b12380f963..96789c43c5 100644
--- a/maven-core/src/main/java/org/apache/maven/toolchain/DefaultToolchain.java
+++ b/maven-core/src/main/java/org/apache/maven/toolchain/DefaultToolchain.java
@@ -88,11 +88,11 @@ implements Toolchain, ToolchainPrivate {
RequirementMatcher matcher = provides.get(key);
if (matcher == null) {
- getLog().debug("Toolchain " + this + " is missing required
property: " + key);
+ getLog().debug("Toolchain {} is missing required property:
{}", this, key);
return false;
}
if (!matcher.matches(requirement.getValue())) {
- getLog().debug("Toolchain " + this + " doesn't match required
property: " + key);
+ getLog().debug("Toolchain {} doesn't match required property:
{}", this, key);
return false;
}
}
diff --git
a/maven-core/src/test/java/org/apache/maven/classrealm/DefaultClassRealmManagerTest.java
b/maven-core/src/test/java/org/apache/maven/classrealm/DefaultClassRealmManagerTest.java
new file mode 100644
index 0000000000..f1908adb42
--- /dev/null
+++
b/maven-core/src/test/java/org/apache/maven/classrealm/DefaultClassRealmManagerTest.java
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.classrealm;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+
+import org.apache.maven.extension.internal.CoreExports;
+import org.apache.maven.model.Model;
+import org.codehaus.plexus.DefaultPlexusContainer;
+import org.codehaus.plexus.PlexusContainer;
+import org.codehaus.plexus.PlexusContainerException;
+import org.codehaus.plexus.classworlds.realm.ClassRealm;
+import org.eclipse.aether.artifact.Artifact;
+import org.junit.jupiter.api.Test;
+import org.mockito.InOrder;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.endsWith;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.calls;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.when;
+
+/**
+ * @author Sebastien Doyon
+ */
+class DefaultClassRealmManagerTest {
+
+ private DefaultClassRealmManager
newDefaultClassRealmManager(PlexusContainer container) {
+ HashSet<String> exportedPackages = new HashSet<String>();
+ exportedPackages.add("group1:artifact1");
+
+ return new DefaultClassRealmManager(
+ container,
+ new ArrayList<ClassRealmManagerDelegate>(),
+ new CoreExports(new ClassRealm(null, "test", null), new
HashSet<String>(), exportedPackages));
+ }
+
+ private List<Artifact> newTestArtifactList() {
+ List<Artifact> artifacts = new ArrayList<Artifact>();
+
+ Artifact artifact = mock(Artifact.class);
+ when(artifact.getFile()).thenReturn(new File(new
File("local/repository"), "some/path"));
+ when(artifact.getGroupId()).thenReturn("group1");
+ when(artifact.getArtifactId()).thenReturn("artifact1");
+ when(artifact.getExtension()).thenReturn("ext");
+ when(artifact.getClassifier()).thenReturn("classifier1");
+ when(artifact.getVersion()).thenReturn("1");
+ artifacts.add(artifact);
+
+ Artifact artifact2 = mock(Artifact.class);
+ when(artifact2.getFile()).thenReturn(null);
+ when(artifact2.getGroupId()).thenReturn("group1");
+ when(artifact2.getArtifactId()).thenReturn("artifact2");
+ when(artifact2.getExtension()).thenReturn("ext");
+ when(artifact2.getClassifier()).thenReturn("classifier1");
+ when(artifact2.getVersion()).thenReturn("1");
+ artifacts.add(artifact2);
+
+ return artifacts;
+ }
+
+ private Model newTestModel() {
+ Model model = new Model();
+ model.setGroupId("modelGroup1");
+ model.setArtifactId("modelArtifact1");
+ model.setVersion("modelVersion1");
+
+ return model;
+ }
+
+ @Test
+ void testDebugEnabled() throws PlexusContainerException {
+ Logger logger = mock(Logger.class);
+ when(logger.isDebugEnabled()).thenReturn(true);
+
+ DefaultClassRealmManager classRealmManager;
+ ClassRealm classRealm;
+
+ InOrder verifier = inOrder(logger);
+
+ PlexusContainer container = new DefaultPlexusContainer();
+
+ try (MockedStatic<LoggerFactory> mockedLoggerFactory =
Mockito.mockStatic(LoggerFactory.class)) {
+ mockedLoggerFactory
+ .when(() ->
LoggerFactory.getLogger(DefaultClassRealmManager.class))
+ .thenReturn(logger);
+
+ classRealmManager = newDefaultClassRealmManager(container);
+ classRealm = classRealmManager.createProjectRealm(newTestModel(),
newTestArtifactList());
+ }
+
+ assertEquals(classRealmManager.getMavenApiRealm(),
classRealm.getParentClassLoader());
+ assertEquals("project>modelGroup1:modelArtifact1:modelVersion1",
classRealm.getId());
+ assertEquals(1, classRealm.getURLs().length);
+ assertThat(classRealm.getURLs()[0].getPath(),
endsWith("local/repository/some/path"));
+
+ verifier.verify(logger, calls(1)).debug("Importing foreign packages
into class realm {}", "maven.api");
+ verifier.verify(logger, calls(1)).debug(" Imported: {} < {}",
"group1:artifact1", "test");
+ verifier.verify(logger, calls(1)).debug(" Excluded: {}",
"group1:artifact2:ext:classifier1:null");
+ verifier.verify(logger, calls(1))
+ .debug("Populating class realm {}",
"project>modelGroup1:modelArtifact1:modelVersion1");
+ verifier.verify(logger, calls(1)).debug(" Included: {}",
"group1:artifact1:ext:classifier1:null");
+ }
+
+ @Test
+ void testDebugDisabled() throws PlexusContainerException {
+ Logger logger = mock(Logger.class);
+ when(logger.isDebugEnabled()).thenReturn(false);
+
+ DefaultClassRealmManager classRealmManager;
+ ClassRealm classRealm;
+
+ InOrder verifier = inOrder(logger);
+
+ PlexusContainer container = new DefaultPlexusContainer();
+
+ try (MockedStatic<LoggerFactory> mockedLoggerFactory =
Mockito.mockStatic(LoggerFactory.class)) {
+ mockedLoggerFactory
+ .when(() ->
LoggerFactory.getLogger(DefaultClassRealmManager.class))
+ .thenReturn(logger);
+
+ classRealmManager = newDefaultClassRealmManager(container);
+ classRealm = classRealmManager.createProjectRealm(newTestModel(),
newTestArtifactList());
+ }
+
+ assertEquals(classRealmManager.getMavenApiRealm(),
classRealm.getParentClassLoader());
+ assertEquals("project>modelGroup1:modelArtifact1:modelVersion1",
classRealm.getId());
+ assertEquals(1, classRealm.getURLs().length);
+ assertThat(classRealm.getURLs()[0].getPath(),
endsWith("local/repository/some/path"));
+
+ verifier.verify(logger, calls(1)).debug("Importing foreign packages
into class realm {}", "maven.api");
+ verifier.verify(logger, calls(1)).debug(" Imported: {} < {}",
"group1:artifact1", "test");
+ verifier.verify(logger, calls(1))
+ .debug("Populating class realm {}",
"project>modelGroup1:modelArtifact1:modelVersion1");
+ verifier.verify(logger, never()).debug(" Included: {}",
"group1:artifact1:ext:classifier1:null");
+ verifier.verify(logger, never()).debug(" Excluded: {}",
"group1:artifact2:ext:classifier1:null");
+ }
+}
diff --git
a/maven-core/src/test/java/org/apache/maven/toolchain/DefaultToolchainTest.java
b/maven-core/src/test/java/org/apache/maven/toolchain/DefaultToolchainTest.java
index faffb3c75d..d6efd846df 100644
---
a/maven-core/src/test/java/org/apache/maven/toolchain/DefaultToolchainTest.java
+++
b/maven-core/src/test/java/org/apache/maven/toolchain/DefaultToolchainTest.java
@@ -92,7 +92,7 @@ class DefaultToolchainTest {
DefaultToolchain toolchain = newDefaultToolchain(model);
assertFalse(toolchain.matchesRequirements(Collections.singletonMap("name",
"John Doe")));
- verify(logger).debug("Toolchain type:TYPE{} is missing required
property: name");
+ verify(logger).debug("Toolchain {} is missing required property: {}",
toolchain, "name");
}
@Test
@@ -103,7 +103,7 @@ class DefaultToolchainTest {
toolchain.addProvideToken("name",
RequirementMatcherFactory.createExactMatcher("Jane Doe"));
assertFalse(toolchain.matchesRequirements(Collections.singletonMap("name",
"John Doe")));
- verify(logger).debug("Toolchain type:TYPE{name = Jane Doe} doesn't
match required property: name");
+ verify(logger).debug("Toolchain {} doesn't match required property:
{}", toolchain, "name");
}
@Test
