gnodet-bot commented on code in PR #398:
URL: https://github.com/apache/maven-filtering/pull/398#discussion_r4103292064
##########
src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java:
##########
@@ -256,6 +265,20 @@ public void filterResources(MavenResourcesExecution
mavenResourcesExecution) thr
Path destinationFile = getDestinationFile(outputDirectory,
targetPath, name, mavenResourcesExecution);
+ // Track destinations already written by a previous <resource>
entry so we
+ // can preserve "first <resource> entry wins" semantics for
overlapping
+ // declarations. The flatten case has its own collision policy
below
+ // (warn-or-throw based on overwrite), so we deliberately do
not suppress
+ // it here.
+ Path destinationPath = destinationFile.toAbsolutePath();
+ boolean firstOccurrence = alreadyCopied.add(destinationPath);
+ if (!firstOccurrence
+ && !mavenResourcesExecution.isFlatten()
+ && !mavenResourcesExecution.isOverwrite()) {
Review Comment:
nit: the comment on L120-121 in the
`overlappingEntriesWithExplicitOverwriteStillReplace` test references
`ChangeDetection.ALWAYS`, but that enum is introduced by PR #397 which hasn't
merged yet. On current `master`, only `isOverwrite()` exists. Worth removing
the forward reference to avoid confusion if this PR merges first.
##########
src/test/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFilteringTest.java:
##########
@@ -1079,4 +1079,48 @@ private List<Path> list(Path file) throws IOException {
private boolean contentEquals(Path p1, Path p2) throws IOException {
return Files.mismatch(p1, p2) < 0;
}
+
+ /**
+ * Regression guard for the maven-resources-plugin issue 471 / GH-333
scenario: when the same
+ * destination is reached by a later <resource> entry while
overwrite is explicitly enabled,
+ * the later entry must still be allowed to replace the earlier one — the
"first entry wins"
+ * skip introduced for the default case must not apply here.
+ */
+ @Test
+ void overlappingEntriesWithExplicitOverwriteStillReplace() throws
Exception {
+ mavenProject.addProperty("repro.value", "REPLACED_BY_FILTERING");
+
+ String unitFilesDir = getBasedir() + "/src/test/units-files/MRP-471";
+
+ Resource filtered = new Resource();
+ filtered.setDirectory(unitFilesDir);
+ filtered.setFiltering(true);
+ filtered.addInclude("config/filtered.xml");
+
+ Resource unfiltered = new Resource();
+ unfiltered.setDirectory(unitFilesDir);
+ unfiltered.setFiltering(false);
+ unfiltered.addInclude("**");
+
+ MavenResourcesExecution execution = new MavenResourcesExecution(
+ Arrays.asList(filtered, unfiltered),
+ outputDirectory,
+ mavenProject,
+ "UTF-8",
+ Collections.emptyList(),
+ Collections.emptyList(),
+ new StubSession());
+ execution.setUseDefaultFilterWrappers(true);
+ execution.setOverwrite(true);
+
+ mavenResourcesFiltering.filterResources(execution);
+
+ Path filteredOut = outputDirectory.resolve("config/filtered.xml");
+ String content = new String(Files.readAllBytes(filteredOut),
StandardCharsets.UTF_8);
+ // With overwrite=true the unfiltered second pass still wins —
preserve the historical
+ // 3.3.1 behaviour for callers that opt in to ChangeDetection.ALWAYS.
Review Comment:
This forward-references `ChangeDetection.ALWAYS` from PR #397 which is still
open. If #398 merges first, this comment will reference a non-existent concept.
Consider rephrasing to just mention `overwrite=true` semantics.
--
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]
