[MPH-217] Ensure packaging is always shown in effective-pom output

[elharo]

Problem: help:effective-pom does not show the <packaging> element when the packaging is not explicitly set in the POM (defaulting to jar). MavenXpp3Writer intentionally omits the <packaging> element when it is null or equals "jar" (the default), so users cannot see the effective packaging value.

Fix: After serialization, detect if the <packaging> element is missing and insert it into the XML output if so. This ensures the effective packaging value is always visible.

Test: Added an integration test (effective-pom-packaging) with a POM that has no <packaging> element, verifying the output contains <packaging>jar</packaging>.

Fixes #332

[pzygielo]

If given project has parent, like

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <parent>
    <groupId>...</groupId>
    <artifactId>...</artifactId>
    <version>0.1-SNAPSHOT</version>
  </parent>
  <groupId>...</groupId>
  <artifactId>..</artifactId>
  <version>0.1-SNAPSHOT</version>
  ...

which </version> will be replaced in l. 206?

[pzygielo]

I updated IT presented here with:

diff --git a/src/it/projects/effective-pom-packaging/pom.xml b/src/it/projects/effective-pom-packaging/pom.xml
index 6622e25..a97aed6 100644
--- a/src/it/projects/effective-pom-packaging/pom.xml
+++ b/src/it/projects/effective-pom-packaging/pom.xml
@@ -21,7 +21,11 @@ under the License.
 
 <project>
   <modelVersion>4.0.0</modelVersion>
-
+<parent>
+<groupId>org.apache.maven</groupId>
+<artifactId>maven-parent</artifactId>
+<version>49</version>
+</parent>
   <groupId>org.apache.maven.its.help</groupId>
   <artifactId>mph-217</artifactId>
   <version>1.0-SNAPSHOT</version>

And here is test result: SUCCESS.

But here is the goal result:

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <parent>
    <groupId>org.apache.maven</groupId>
    <artifactId>maven-parent</artifactId>
    <version>49</version>
    <packaging>jar</packaging>
  </parent>
  <groupId>org.apache.maven.its.help</groupId>
  <artifactId>mph-217</artifactId>
  <version>1.0-SNAPSHOT</version>
  <packaging>jar</packaging>

This is of course non-sense for parent.

And, perhaps according to String.replace javadoc (Replaces each substring of this string that matches the literal target sequence), every </version>, including plugins' - has explicit <packaging>jar</packaging> companion now.

I don't think it's correct...

[pzygielo]

To clarify above - even without any modification to this PR - manual inspection of target/it/effective-pom-packaging/build.log shows that too much is done to the effective pom.

Perhaps src/it/projects/effective-pom-packaging/verify.groovy could be fixed to check that only expected changes are there (i.e. no unexpected changes are).

[elharo]

Good point and a good reminder of why regexes are dangerous when processing XML. There are other failure modes here too. I might take a whirl at fixing this with some real XML.

[Copilot]

Pull request overview

This PR addresses MPH-217 / #332 by ensuring help:effective-pom always shows an explicit <packaging> element, even when the project relies on Maven’s default packaging (jar).

Changes:

• Post-processes the serialized effective POM output to add a <packaging> element when it would otherwise be omitted.
• Adds a new integration test project (effective-pom-packaging) to verify <packaging>jar</packaging> appears when packaging is not specified in the input POM.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/main/java/org/apache/maven/plugins/help/EffectivePomMojo.java	Adds logic intended to inject <packaging> into effective-pom output when omitted by serialization.
src/it/projects/effective-pom-packaging/verify.groovy	New IT assertion checking the effective-pom output includes <packaging>jar</packaging>.
src/it/projects/effective-pom-packaging/pom.xml	New IT project POM without a <packaging> element to exercise the default behavior.
src/it/projects/effective-pom-packaging/invoker.properties	Runs the effective-pom goal for the new IT project.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ascheman]

Agree with @pzygielo's diagnosis — won't re-litigate the String.replace("</version>", …) issue. Since you're planning to redo this with real XML, a few things to make sure the rewrite covers, plus one gotcha:

Beyond the parent case already shown:

• Multi-module reactors — at an aggregator, effective-pom emits all modules under one <projects> root, so there are N top-level <project> elements (each needs its own packaging) and many more </version> to accidentally hit. The fix has to target each project root, not "the" project.
• Schema validity — <packaging> is only valid directly under <project> (after <version>), not inside <parent>; the current output is schema-invalid there, not just noisy. Selecting the project root element(s) fixes both.

Gotcha: model.setPackaging("jar") before serialization won't help — MavenXpp3Writer omits packaging when it's "jar", so it'd be dropped again. A DOM post-process (for each project root lacking a direct-child <packaging>, insert after <version>) is probably cleanest.

Test: the current verify.groovy (find(' <packaging>jar</packaging>')) would pass even with the duplication bug. Worth an IT matrix: (a) a <parent> project asserting no <packaging> inside <parent>; (b) a multi-module reactor asserting one project-level <packaging> per module and none on deps/plugins; (c) explicit non-jar packaging shown once. The count/negative assertions are what catch this.