gnodet commented on code in PR #13114:
URL: https://github.com/apache/maven/pull/13114#discussion_r4018886583
##########
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java:
##########
@@ -516,16 +518,86 @@ private static boolean
isExternalModelBuildingRequest(ModelBuildingRequest reque
}
/**
- * Returns the profiles from the given list whose activation does not
depend on a file or a
- * property. Profiles activated by JDK version, operating system, or marked
- * {@code activeByDefault} are unaffected, since those conditions are a
function of the build
- * platform rather than of the model content.
+ * Returns a sandboxed {@link ProfileActivationContext} for evaluating
profiles in
+ * repository-resolved (external) models — dependency POMs, parent POMs,
and imported BOMs.
+ * <p>
+ * The sandboxed context preserves system properties (so JDK/OS activation
works) and
+ * merges the POM's own {@code <properties>} into the system properties
map so that
+ * property-activated profiles that depend on POM-declared values still
work.
+ * User properties (consumer {@code -D} flags) are suppressed because they
were not
+ * set for the dependency and must not accidentally activate its profiles.
+ * File-based profiles are pre-filtered via {@link
#withoutFileActivation(List)} before
+ * reaching this context, so {@code getProjectDirectory()} is not relied
on for file checks.
+ * <p>
+ * Model properties are merged into system properties (with system
properties taking
+ * precedence) rather than changing the {@code PropertyProfileActivator}
lookup chain,
+ * because changing the activator would affect ALL profile evaluations —
including the
+ * build's own project — which can cause unintended profile activation
when a POM declares
+ * a property that matches a profile's activation condition.
+ *
+ * @param delegate the original full context for this model build
+ * @return a sandboxed context suitable for external model profile
activation
*/
- private static List<Profile>
withoutFileAndPropertyActivation(List<Profile> profiles) {
+ private static ProfileActivationContext
externalActivationContext(ProfileActivationContext delegate) {
+ return new ProfileActivationContext() {
+ @Override
+ public List<String> getActiveProfileIds() {
+ return delegate.getActiveProfileIds();
+ }
+
+ @Override
+ public List<String> getInactiveProfileIds() {
+ return delegate.getInactiveProfileIds();
+ }
+
+ /**
+ * System properties merged with project properties (system wins
on conflict).
+ * This makes POM-declared properties visible to the
PropertyProfileActivator
+ * without modifying the activator's lookup chain for non-external
models.
+ */
+ @Override
+ public Map<String, String> getSystemProperties() {
+ Map<String, String> projectProps =
delegate.getProjectProperties();
+ if (projectProps == null || projectProps.isEmpty()) {
+ return delegate.getSystemProperties();
+ }
+ Map<String, String> merged = new HashMap<>(projectProps);
+ merged.putAll(delegate.getSystemProperties()); // system wins
+ return merged;
Review Comment:
Fixed in f80e41d33a. Pre-computed the merged map before the anonymous class
— one allocation per external model instead of one per `getSystemProperties()`
call. Also wrapped the result in `Collections.unmodifiableMap()` to make the
no-mutation contract explicit.
--
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]
