juulhobert opened a new issue, #11899: URL: https://github.com/apache/maven/issues/11899
### Affected version 4.0-rc-5 ### Bug description `DefaultSettingsXmlFactory.write()` always emits `InputLocation` comments (e.g. `<!-- /path/to/settings.xml:163 -->`) because it creates a bare `new SettingsStaxWriter()` whose `addLocationInformation` field defaults to `true`, and never calls `setAddLocationInformation(false)`. It also completely ignores the `inputLocationFormatter` from the `XmlWriterRequest`. This is inconsistent with `DefaultModelXmlFactory.write()`, which correctly defaults location tracking to `false` and only enables it when an explicit `inputLocationFormatter` is provided. ## Affected Goal `help:effective-settings` (Theoretical, there is no Maven Help plugin version which is Maven4 only. See reproducer for a practical example) ## Reproduction [Reproducer.zip](https://github.com/user-attachments/files/26513846/Reproducer.zip) A standalone reproducer plugin is available in `Reproducer/` that calls `SettingsXmlFactory.write()` directly without an `inputLocationFormatter`: ```bash cd Reproducer ./mvnw install ./mvnw org.apache.maven.its:settings-location-reproducer:1.0-SNAPSHOT:dump-settings ``` The output contains inline comments with file paths and line numbers after each element: ```xml <mirror> <mirrorOf>external:http:*</mirrorOf> <!-- /path/to/settings.xml:163 --> <name>Pseudo repository</name> <!-- /path/to/settings.xml:164 --> </mirror> ``` ## Expected Behaviour The serialized XML should be clean — no location comments — matching the output produced by Maven 3's `SettingsXpp3Writer` and by Maven 4's `DefaultModelXmlFactory` (which respects the `XmlWriterRequest` contract): ```xml <mirror> <mirrorOf>external:http:*</mirrorOf> <name>Pseudo repository</name> </mirror> ``` Location comments should only appear when the caller explicitly provides an `inputLocationFormatter` via `XmlWriterRequest.builder().inputLocationFormatter(…)`. ## Observed Behaviour Every element that carries `InputLocation` metadata gets a trailing `<!-- /path/to/settings.xml:NNN -->` comment. There is no way for the caller to suppress them through the `XmlWriterRequest` API because `DefaultSettingsXmlFactory` ignores all request properties except `content`, `writer`, and `outputStream`. ## Suspected Cause **File**: `impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSettingsXmlFactory.java` **Method**: `write(XmlWriterRequest<Settings>)` (lines 71–90) The current code: ```java public void write(XmlWriterRequest<Settings> request) throws XmlWriterException { nonNull(request, "request"); Settings content = nonNull(request.getContent(), "content"); OutputStream outputStream = request.getOutputStream(); Writer writer = request.getWriter(); if (writer == null && outputStream == null) { throw new IllegalArgumentException("writer or outputStream must be non null"); } try { if (writer != null) { new SettingsStaxWriter().write(writer, content); } else { new SettingsStaxWriter().write(outputStream, content); } } catch (Exception e) { throw new XmlWriterException( "Unable to write settings: " + getMessage(e), getLocation(e), e); } } ``` Problems: 1. `new SettingsStaxWriter()` — the constructor sets `addLocationInformation = true` (verified in bytecode: `iconst_1` → `putfield addLocationInformation`). 2. `setAddLocationInformation(false)` is never called. 3. `request.getInputLocationFormatter()` is never read. Compare with `DefaultModelXmlFactory.write()` (lines 154–177) which does it correctly: ```java MavenStaxWriter xmlWriter = new MavenStaxWriter(); xmlWriter.setAddLocationInformation(false); // ← defaults OFF Function<Object, String> formatter = request.getInputLocationFormatter(); if (formatter != null) { xmlWriter.setAddLocationInformation(true); // ← only ON with formatter Function<InputLocation, String> adapter = formatter::apply; xmlWriter.setStringFormatter(adapter); } ``` ## Workaround None known. All plugin-side workarounds are either brittle or incorrect: - Stripping all XML comments post-serialization would destroy legitimate user comments. - Regex-filtering location-pattern comments is fragile — user comments could match. - Rebuilding the entire `Settings` model tree with `newBuilder(obj, false)` to strip `InputLocation` data is excessively complex, untested for deep nesting, and couples the plugin to internal model implementation details. -- 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]
