gnodet commented on code in PR #13158:
URL: https://github.com/apache/maven/pull/13158#discussion_r4037986100
##########
compat/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java:
##########
@@ -515,16 +517,93 @@ 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) {
+ // Pre-compute the merged system+project properties once per external
model.
+ // getSystemProperties() may be called multiple times per profile
(e.g. OperatingSystemProfileActivator
+ // calls it 3× for name/arch/version), so allocating a new HashMap on
every call is O(deps × profiles ×
+ // |systemProperties|). Computing it eagerly here keeps the anonymous
class allocation-free.
+ final Map<String, String> mergedSystemProps;
+ Map<String, String> projectProps = delegate.getProjectProperties();
+ if (projectProps == null || projectProps.isEmpty()) {
+ mergedSystemProps = delegate.getSystemProperties();
+ } else {
+ Map<String, String> merged = new HashMap<>(projectProps);
Review Comment:
Fixed in f321eab98f: wrapped with `Collections.unmodifiableMap()` on the
empty-projectProps branch.
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java:
##########
@@ -1696,25 +1697,38 @@ private void addActivePomProfiles(String modelId,
List<Profile> activePomProfile
private List<Profile> getActiveProfiles(
Review Comment:
Fixed in f321eab98f: added `hasFileConditionExpression()` helper and
extended the pre-filter to also exclude condition profiles whose expression
contains `exists(` or `missing(`. Added test: `profile.missing.condition`
profile in `resolved-model-with-profiles.xml` asserts it fires in BUILD_PROJECT
(path genuinely doesn't exist) but is suppressed in CONSUMER_DEPENDENCY.
--
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]
