Skip to content

fix(gettext): coalesce duplicate file changes in incremental msg* tasks#188

Open
vlsi wants to merge 3 commits into
masterfrom
claude/infallible-poitras-dea1d4
Open

fix(gettext): coalesce duplicate file changes in incremental msg* tasks#188
vlsi wants to merge 3 commits into
masterfrom
claude/infallible-poitras-dea1d4

Conversation

@vlsi

@vlsi vlsi commented May 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • Downstream consumers (e.g. pgjdbc's generateGettextSources) sometimes lost a locale's output: the per-locale messages_<lang>.java was missing from build/gettext/generate_java_resources/po/..., even though <lang>.po was present and unmodified. --rerun-tasks papered it over; the next incremental run reintroduced the loss.

  • Root cause: when poFiles aggregates overlapping providers (e.g. files(sourceSets.main.get().allSource).filter { …".po" } plus feature variants like registerFeature("sspi")/("osgi") reusing sourceSets["main"]), Gradle can report the same physical .po file as both ADDED and REMOVED in one invocation. The task body in MsgAttribTask.run (and MsgMergeTask/MsgFmtTask) iterates events in order, so msgattrib writes outDir/ru.po and the following REMOVED branch immediately deletes it. The empty outDir entry cascades downstream and a locale silently disappears.

    Trace (--info):

    Task ':…:remove_obsolete_translations' is not up-to-date because:
      Input property 'poFiles' file …/ru.po has been added.
      Input property 'poFiles' file …/ru.po has been removed.
    

Fix

Introduce coalesceFileChanges(changes, currentSourceFiles) in FileChangeCoalescing.kt and route the three incremental tasks (MsgAttribTask, MsgMergeTask, MsgFmtTask) through it. Per file.absolutePath:

  • non-FILE entries are dropped,
  • ADDED/MODIFIED wins over REMOVED (collapses the spurious pair to a single ADDED/MODIFIED),
  • a pure-REMOVED event is suppressed when the file is still present in the resolved poFiles.files snapshot.

Insertion order is preserved, so the processing order matches the previous behavior.

Tests

Added FileChangeCoalescingTest (8 unit cases — all green):

  • ADDED → REMOVED collapses to ADDED
  • REMOVED → ADDED collapses to ADDED
  • MODIFIED wins over REMOVED
  • pure REMOVED suppressed when the file is still in the snapshot
  • pure REMOVED kept when the file is absent from the snapshot
  • non-FILE (DIRECTORY) entries dropped
  • distinct files reported independently
  • insertion order preserved across distinct files

I did not add an end-to-end integration test that wires duplicate providers through a real GradleRunner: msgattrib/msgmerge/msgfmt would have to be installed on the test host, and the plugin had no test sources before this PR. The reported regression is purely the duplicate-event handling, which the unit tests cover exactly.

Test plan

  • ./gradlew :plugins:gettext-plugin:test --rerun-tasks — 8/8 green
  • ./gradlew :plugins:gettext-plugin:build — green
  • Run pgjdbc repro from the bug report (./gradlew :postgresql:generateGettextSources --rerun-tasks, delete one messages_<lang>.java, re-run with --info) and confirm the locale output is regenerated and no spurious ADDED + REMOVED pair drops it.

🤖 Generated with Claude Code

vlsi and others added 3 commits May 25, 2026 22:22
When poFiles aggregates overlapping providers (e.g. allSource plus feature
variants reusing the main source set), Gradle can report the same physical
file as both ADDED and REMOVED in a single incremental invocation. The
previous loop processed events in order, so msgattrib/msgmerge/msgfmt would
write the output and then immediately delete it. Downstream tasks then saw
the empty output and dropped their own per-locale outputs, leaving e.g.
messages_ru.java missing until --rerun-tasks was used.

Introduce coalesceFileChanges() and route the three incremental tasks
through it. Per file.absolutePath: ADDED/MODIFIED wins over REMOVED, and a
pure-REMOVED event is suppressed when the file is still present in the
resolved poFiles snapshot. Non-FILE entries are dropped centrally.

Add unit tests covering the eight relevant cases.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…heck

Per review feedback. Reorder the predicate so `existing?.changeType` carries
its own null-safety instead of relying on smart-cast after an `existing == null`
short-circuit (which would make `?.` redundant). Semantics unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…he safe-call

Per review feedback. With `existing == null ||` on the left, Kotlin smart-casts
`existing` to non-null in the right operand, so the `?.` is no longer needed.
Semantics unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant