Skip to content

[Backport maven-4.0.x] Fix #12931: do not rewrite unmodified POMs in mvnup apply - #12972

Merged
gnodet merged 1 commit into
maven-4.0.xfrom
backport/12938-to-maven-4.0.x
Aug 31, 2026
Merged

[Backport maven-4.0.x] Fix #12931: do not rewrite unmodified POMs in mvnup apply#12972
gnodet merged 1 commit into
maven-4.0.xfrom
backport/12938-to-maven-4.0.x

Conversation

@gnodet

@gnodet gnodet commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Backport of #12938 to maven-4.0.x.

Summary

mvnup apply always logged Saving modified POMs... and wrote every discovered POM, even when every strategy reported 0 POM(s) modified. That both misleads the user and rewrites files that did not change.

UpgradeResult already tracks modifiedPoms. Apply now:

  • writes only those POMs
  • skips the save report entirely when the set is empty

Fixes #12931

* Fix #12931: do not rewrite unmodified POMs in mvnup apply

mvnup apply always logged "Saving modified POMs..." and wrote every
discovered POM, even when no strategy reported changes. Save only the
POMs in UpgradeResult.modifiedPoms and skip the save report when that
set is empty.

* Don't call saveModifications when no POM was modified

Skip the save path in execute when modifiedPoms is empty, instead of
returning early inside saveModifications.
@gnodet gnodet added this to the 4.0.0 milestone Aug 31, 2026
@gnodet gnodet added the bug Something isn't working label Aug 31, 2026
@gnodet gnodet modified the milestones: 4.0.0, 4.0.0-rc-7 Aug 31, 2026
@gnodet
gnodet merged commit d17092e into maven-4.0.x Aug 31, 2026
24 checks passed
@gnodet
gnodet deleted the backport/12938-to-maven-4.0.x branch August 31, 2026 08:30

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

✅ Looks Good

Clean backport of #12938 (merged to master) to maven-4.0.x. The diff is byte-for-byte identical to the original.

Technical notes:

  • The change from returning int to UpgradeResult in doUpgrade correctly exposes modifiedPoms() to the caller without breaking Apply or Check subclasses.
  • The exception-handling path constructs UpgradeResult.failure() with empty modifiedPoms, preventing saveModifications from being called on failure — correct behavior.
  • Tests use FileTime comparison to verify unmodified POMs are not rewritten (not just that the method isn't called) — robust assertion strategy.
  • No backport to 3.x branches needed — mvnup is a Maven 4 feature.

🤖 This review was generated by ForgeBot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants