Conversation
There was a problem hiding this comment.
Pull request overview
Updates POM XML namespace emission in PomWriter to use the canonical Maven 4.0.0 namespace URI, ensuring generated POMs declare the standard namespace.
Changes:
- Hardcode the POM XML namespace to
http://maven.apache.org/POM/4.0.0. - Update the
schemaLocationattribute’s namespace portion to matchhttp://maven.apache.org/POM/4.0.0.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| root.setAttribute( | ||
| "schemaLocation", | ||
| "http://maven.apache.org/POM/" + modelVersion + " http://maven.apache.org/maven-v" | ||
| "http://maven.apache.org/POM/4.0.0" + " http://maven.apache.org/maven-v" |
There was a problem hiding this comment.
This concatenation joins two string literals (".../4.0.0" + " http://...") which is harder to read than a single literal. Collapsing it into one string (or building it from constants) would improve readability.
| "http://maven.apache.org/POM/4.0.0" + " http://maven.apache.org/maven-v" | |
| "http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v" |
|
FYI CI looks broken: Error: Failed to execute goal com.diffplug.spotless:spotless-maven-plugin:2.43.0:check (default) on project maven-shade-plugin: Execution default of goal com.diffplug.spotless:spotless-maven-plugin:2.43.0:check failed: An API incompatibility was encountered while executing com.diffplug.spotless:spotless-maven-plugin:2.43.0:check: java.lang.NoSuchMethodError: 'java.util.Queue com.sun.tools.javac.util.Log$DeferredDiagnosticHandler.getDiagnostics()' |
|
@elharo did you rebase your branch with the latest master version? |
|
done |
|
Thanks @elharo. I dug into this because it touches the dependency-reduced-POM writer ( For today's 4.0.0 POMs this is a no-op — with For a 4.1.0 build model it produces a self-contradictory POM. The <project xmlns="http://maven.apache.org/POM/4.0.0"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_1_0.xsd">
<modelVersion>4.1.0</modelVersion>
</project>— a 4.0.0 namespace paired with a 4.1.0 schema and a 4.1.0 The design question underneath: should shade's dependency-reduced POM be a 4.0.0 (consumer-style) POM even when the build POM is 4.1.0? There's a real case for it — the DRP is consumed by downstream projects (shaded libraries), so broad 4.0.0 compatibility is desirable. But if that's the goal, it needs a genuine downgrade ( @gnodet — since you own the consumer-POM design: for a 4.1.0 build POM, should shade emit the DRP in the build model version (4.1.0) and let core's consumer-POM transform produce the 4.0.0 consumer, or should the DRP itself be a fully-downgraded 4.0.0 POM? That answer decides the right shape here. As written the change is a no-op for 4.0.0 and inconsistent for 4.1.0, so I don't think it should merge in this form. Happy to share the small probe I used. |
No description provided.