[MNG-5359] Do not activate pluginManagement executions through lifecycle bindings

[goutamadwant]

Fixes #6918.

Problem

When lifecycle bindings introduce a plugin that also appears in pluginManagement, Maven currently clones and merges the entire managed plugin. This makes pluginManagement active by copying managed executions into the lifecycle plugin, even though the plugin was never declared in build/plugins.

On current master, a maven-clean-plugin execution named custom-clean, declared only in pluginManagement and bound to initialize, is executed by mvn initialize.

Change

• Use the lifecycle-supplied plugin as the merge base.
• Overlay only the managed plugin version and configuration, preserving their existing dominance.
• Keep lifecycle executions and all other plugin-level fields unchanged.
• Apply the same behavior to the legacy mutable model and the Maven 4 immutable model.

This preserves the existing behavior where lifecycle plugins obtain their version and configuration from pluginManagement, while keeping managed executions passive until the plugin is explicitly declared.

Tests

• Added focused merger tests for both model implementations. They verify:
  • managed version and configuration are applied;
  • managed configuration wins on conflicting keys;
  • lifecycle executions and input locations are preserved;
  • managed executions, dependencies, extensions, and inheritance flags are not copied.
• Added MavenITmng5359PluginManagementExecutionTest with two end-to-end paths:
  • a management-only execution remains inactive;
  • the same execution activates when the plugin is explicitly declared.
• Ran mvn -Prun-its -Dits.test=MavenITmng5359PluginManagementExecutionTest verify successfully across all 90 reactor modules.
• Verified the built CLI directly: management-only initialize executes no clean goal, explicit-plugin initialize executes custom-clean, and the normal clean lifecycle still executes only default-clean.

Compatibility and scope

This change is limited to lifecycle-binding injection and does not modify public APIs. Explicit build/plugins declarations continue to receive normal plugin-management merging. The implementation replaces the previous clone/full-merge path with one clone or builder plus two field merges.

Following this checklist to help us incorporate the contribution quickly and easily:

• This pull request addresses one issue without unrelated changes.

• The description explains what the pull request does, how, and why.

• The commit has a meaningful subject line and body.

• Behavioral unit tests fail without the runtime change.

• mvn verify passed as part of the broader mvn -Prun-its verify run.

• The targeted Core IT passed successfully.

• I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

• In any other case, I have filed an Apache Individual Contributor License Agreement.

[gnodet]

Please provide an IT that reproduces the problem, it's way easier to understand the exact problem.

[goutamadwant]

Thanks @gnodet. The PR includes MavenITmng5359PluginManagementExecutionTest, which reproduces the pluginManagement-only case and includes an explicit plugin declaration as a positive control.

While validating the fix, I found that filtering all managed executions regressed MNG-4344. The follow-up now filters only executions bound to a different lifecycle while retaining same-lifecycle and phase-less executions.

Validated with the focused lifecycle injector tests and both MavenITmng5359PluginManagementExecutionTest and MavenITmng4344ManagedPluginExecutionOrderTest. Let me know. thanks!

[gnodet]

Thanks for tackling this long-standing issue. The approach of filtering cross-lifecycle managed executions while preserving same-lifecycle ones is sound and addresses MNG-5359 without regressing MNG-4344. Tests are solid with good positive/negative controls.

A few observations below.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet]

💡 Performance: getPhaseToLifecycleMap() is called on every injectLifecycleBindings invocation, creating a new HashMap and a new LifecycleBindingsMerger each time. The previous code cached a single LifecycleBindingsMerger as a field.

Since the phase-to-lifecycle map doesn't change after startup, this could be computed once in the constructor:

Suggested change

	new LifecycleBindingsMerger(getPhaseToLifecycleMap()).merge(model, lifecycleModel);
	new LifecycleBindingsMerger(phaseToLifecycleMap).merge(model, lifecycleModel);

…with phaseToLifecycleMap as a final field initialized in the constructor. Not critical — model building isn't in a tight loop — but it's a free optimization.

[gnodet]

📝 Edge case to consider: When managedPhase is a custom/unknown phase not in phaseToLifecycleMap, managedLifecycle is null and the logic falls back to exact phase matching only. This means a managed execution bound to a custom phase (e.g. from a lifecycle extension) will be filtered out unless a lifecycle execution is bound to the exact same phase.

Is this the intended behavior? It seems reasonable (err on the side of not activating unknown phases), but worth documenting as a conscious design decision, especially since custom lifecycle extensions exist in the wild.

[gnodet]

📝 Consistency with legacy model: The legacy model's getPhaseToLifecycleMap() delegates to DefaultLifecycles.getPhaseToLifecycleMap(), while the Maven 4 model implementation computes the map directly from LifecycleRegistry including aliases. This means the two implementations might produce different maps if alias handling differs.

This is likely fine (the legacy model doesn't have Maven 4 lifecycle aliases), but a comment explaining the difference would help future maintainers.

[gnodet]

💡 Nit: The test verifies the result has 3 executions and checks the expected set, but doesn't verify that the filtered execution (managed-initialize) is absent. While the Set.of(...) assertion implicitly covers this (3 elements, none is managed-initialize), an explicit assertFalse would make the intent clearer:

assertFalse(result.getExecutions().stream()
        .anyMatch(e -> "managed-initialize".equals(e.getId())),
        "Cross-lifecycle managed execution should be filtered out");

[gnodet]

💡 Suggestion: Both test phases could use separate test methods (e.g., testManagedExecutionNotActivatedWithoutDeclaration and testManagedExecutionActivatedWithExplicitDeclaration) for clearer test isolation and failure reporting. If one phase fails, you immediately know which scenario broke.

Also, the test uses package as the target phase, but the managed execution is also bound to package — so the test verifies that a managed clean plugin execution bound to package (a default lifecycle phase) is NOT activated when the clean plugin is introduced only via lifecycle bindings (clean lifecycle). This is a good cross-lifecycle test. A brief comment explaining this would help readability.