gnodet-bot commented on code in PR #13547:
URL: https://github.com/apache/maven/pull/13547#discussion_r4293539759
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Constants.java:
##########
@@ -790,5 +790,18 @@ public final class Constants {
@Config(type = "java.lang.Boolean", defaultValue = "false")
public static final String MAVEN_MODEL_DEPENDENCY_INTERPOLATION_FULL =
"maven.model.dependencyInterpolation.full";
+ /**
+ * System property to disable the {@code site} lifecycle.
+ * When set to {@code true} via {@code
-Dmaven.site.lifecycle.disabled=true}, the {@code site}
+ * lifecycle is excluded from the default lifecycles, so Maven will not
resolve or bind the
+ * {@code maven-site-plugin} unless it is explicitly configured in the POM.
+ * This is a JVM system property only (read via {@link
Boolean#getBoolean(String)});
+ * it is not resolved through Maven's property mechanism (pom.xml,
profiles, settings.xml).
+ *
+ * @since 4.0.0
+ */
Review Comment:
⚠️ **Missing `readOnly = true`**: All 7 existing `SYSTEM_PROPERTIES`-sourced
`@Config` annotations in this file include `readOnly = true` — this one
doesn't. JVM system properties are set at startup and are effectively read-only
from Maven's perspective. Add `readOnly = true` for consistency:
```suggestion
@Config(type = "java.lang.Boolean", defaultValue = "false", source =
Config.Source.SYSTEM_PROPERTIES, readOnly = true)
```
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLifecycleRegistry.java:
##########
@@ -154,9 +156,12 @@ public Stream<Lifecycle> stream() {
lookup.lookupList(LifecycleProvider.class).stream()
.flatMap(p -> p.provides().stream())
.forEach(lf -> result.put(lf.id(), lf));
- return result.values().stream();
+ stream = result.values().stream();
+ } else {
+ stream =
providers.stream().map(ExtensibleEnumProvider::provides).flatMap(Collection::stream);
}
- return
providers.stream().map(ExtensibleEnumProvider::provides).flatMap(Collection::stream);
+ return stream.filter(
+ lf ->
!Boolean.getBoolean(Constants.MAVEN_SITE_LIFECYCLE_DISABLED) ||
!Lifecycle.SITE.equals(lf.id()));
Review Comment:
⚠️ **`Boolean.getBoolean()` re-evaluated on every call**: `stream()` is
called per-project during the build. The [3.10.x backport (PR
#11970)](https://github.com/apache/maven/pull/11970) correctly caches this in a
`private static final boolean` field. Cache it here too:
```suggestion
lf -> !SITE_DISABLED || !Lifecycle.SITE.equals(lf.id()));
```
And add near the top of the class:
```java
private static final boolean SITE_DISABLED =
Boolean.getBoolean(Constants.MAVEN_SITE_LIFECYCLE_DISABLED);
```
--
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]
