You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@maven.apache.org by "elharo (via GitHub)" <gi...@apache.org> on 2026/08/20 12:17:31 UTC

[PR] Fix MavenResourcesExecution.copyOf() to copy missing fields [maven-filtering]

elharo opened a new pull request, #374:
URL: https://github.com/apache/maven-filtering/pull/374

   Fixes #350
   
   `MavenResourcesExecution.copyOf()` was missing four fields:
   - `flatten`
   - `propertiesEncoding`
   - `delimiters`
   - `interpolatorCustomizer`
   
   These fields were silently dropped when `BaseFilter` clones the execution object via `copyOf()`, causing unexpected behavior when the copied instance was used for filtering.
   
   This fix adds the missing field copies and includes tests to verify all four fields are preserved.
   


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix MavenResourcesExecution.copyOf() to copy missing fields [maven-filtering]

Posted by "elharo (via GitHub)" <gi...@apache.org>.

elharo commented on PR #374:
URL: https://github.com/apache/maven-filtering/pull/374#issuecomment-5361893379

   > do we want to guard InterpolatorCustomizer against broken reuse if not stateless somehow? this one can be contextual and copying it can lead to a broken runtime - not in the plugin I'm thinking of but as a shared lib it is, or is a javadoc comment sufficient?
   
   Good point. I'll need to think about that. Possibly we should not copy it here. Possibly it should not be a field at all. Possibly it should be stateless. Maybe something else? I don't know yet. 


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix MavenResourcesExecution.copyOf() to copy missing fields [maven-filtering]

Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.

rmannibucau commented on PR #374:
URL: https://github.com/apache/maven-filtering/pull/374#issuecomment-5362944193

   my first thought was "we shouldn't copy the full model but a "view" but it can make the copy useless too so maybe copyModel() and let the caller copy the dynamic parts if relevant? - thinking out loud


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix MavenResourcesExecution.copyOf() to copy missing fields [maven-filtering]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.

gnodet commented on PR #374:
URL: https://github.com/apache/maven-filtering/pull/374#issuecomment-5813758037

   The history confirms this is exactly a case of fields added after the `copyOf()` fix was written:
   
   - The `copyOf()` method was originally fixed in June 2019 (fixing the bug where it was copying from `mre` instead of `this`)
   - `delimiters` actually predates even that fix but was silently missed in 2019
   - `flatten` was added January 2020 ([MRESOURCES-250])
   - `propertiesEncoding` was added July 2020 ([SHARED-934])
   - `interpolatorCustomizer` was added August 2024 ([MSHARED-1412])
   
   Each time a new field was added, nobody updated `copyOf()`. Thanks for catching all four!
   
   Regarding @rmannibucau's comment about `interpolatorCustomizer`: since the field is explicitly set by the caller on the original execution object, copying the reference is the correct behavior — if a consumer has configured a customizer, they expect the clone to honor it too. A javadoc note would be welcome but not a blocker.


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: copy missing fields in MavenResourcesExecution.copyOf() [maven-filtering]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.

gnodet merged PR #374:
URL: https://github.com/apache/maven-filtering/pull/374


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org