Skip to content

Use Maven#removeTestCompileSourceRoot instead of Iterator#remove#5258

Merged
laeubi merged 1 commit intoeclipse-tycho:mainfrom
laeubi:use_remove_isntead_of_iterator
Aug 28, 2025
Merged

Use Maven#removeTestCompileSourceRoot instead of Iterator#remove#5258
laeubi merged 1 commit intoeclipse-tycho:mainfrom
laeubi:use_remove_isntead_of_iterator

Conversation

@laeubi
Copy link
Member

@laeubi laeubi commented Aug 27, 2025

FYI @hd42 @akurtakov

Fixes warnings:

[WARNING] Direct modification of testCompileSourceRoots through iterator.remove() is deprecated
 and will not work in Maven 4.0.0. Please use the add/remove methods instead.
 If you're using a plugin that causes this warning, please upgrade to the latest version and report an issue if the 
warning persists. To disable these warnings, set -Dmaven.project.sourceRoots.warningsDisabled=true
on the command line, in the .mvn/maven.config file, or in project POM properties.

@laeubi laeubi enabled auto-merge (rebase) August 27, 2025 11:49
@laeubi laeubi disabled auto-merge August 27, 2025 12:25
@laeubi
Copy link
Member Author

laeubi commented Aug 27, 2025

This will require Maven 3.9.11, so I'd like to delay it until after the Tycho 5.0.0 release.

@laeubi laeubi force-pushed the use_remove_isntead_of_iterator branch from e40734d to 693e44d Compare August 27, 2025 12:33
@github-actions
Copy link

github-actions bot commented Aug 27, 2025

Test Results

1 008 files  ±0  1 008 suites  ±0   4h 54m 18s ⏱️ - 39m 4s
1 293 tests ±0  1 272 ✅ ±0  20 💤 ±0  1 ❌ ±0 
3 879 runs  ±0  3 814 ✅ ±0  64 💤 ±0  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit 2485564. ± Comparison against base commit db7744f.

♻️ This comment has been updated with latest results.

for (Iterator<String> iterator = testCompileSourceRoots.iterator(); iterator.hasNext();) {
String testCompileRoot = iterator.next();
private void removeDuplicateTestCompileRoot(File sourceFolder, MavenProject project) {
for (String testCompileRoot : project.getTestCompileSourceRoots()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we must first copy the list, the implicit iterator of the for-each loop does not play well together with calling remove() on the same list instance.

-for (String testCompileRoot : project.getTestCompileSourceRoots()) {
+for (String testCompileRoot : new ArrayList<>(project.getTestCompileSourceRoots())) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sratz while this is true in general and I had the same feeling before when adapting the code, we immediately return when finding a match here so it is okay in this case.

@laeubi laeubi force-pushed the use_remove_isntead_of_iterator branch from 693e44d to 2485564 Compare August 28, 2025 06:47
@laeubi laeubi enabled auto-merge (rebase) August 28, 2025 06:49
@laeubi laeubi merged commit c3b6ba6 into eclipse-tycho:main Aug 28, 2025
13 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants