slachiewicz commented on PR #881: URL: https://github.com/apache/incubator-xtable/pull/881#issuecomment-5252059909
Rebased onto main, which now carries #884. That moved `lombok-maven-plugin` into the `release` profile, so the JDK 21+ fix here moved with it instead of applying at the top level. That makes one claim in the description above stale: delombok no longer runs on ordinary builds, so a contributor on a current JDK never reaches it. What the override now buys is the ability to *cut a release* on a modern JDK. Since the plugin only runs under a profile now, I checked whether release artifacts can drift from what CI builds. They cannot — `xtable-api`, JDK 26, same commit packaged both ways: | Artifact | Normal build | `-DdeployArtifacts=true` | |---|---|---| | Main jar | 113 entries | 113 entries, every `.class` byte-identical | | Sources jar | not produced | byte-identical to `src/main/java` | | Javadoc jar | not produced | 242 HTML files, documents `builder`/`toBuilder` | | `target/delombok` | does not exist | 63 files, read only by javadoc | `<addOutputDirectory>false</addOutputDirectory>` is what makes that hold: delombok output never becomes a compile source root, and only `maven-javadoc-plugin` reads it, through `<sourcepath>`. Worth treating as load-bearing — flipping it to `true` would quietly start compiling release jars from delomboked sources while CI keeps compiling the annotated originals. Which raises a question I would rather have answered than assume: **is delombok meant to serve javadoc only, or should the sources jar ship delomboked sources as well?** Today it is javadoc only. Packing delomboked sources would give consumers the generated builders and getters when they attach sources in an IDE. The costs are that the sources jar would no longer match git, and its line numbers would no longer agree with the bytecode's `LineNumberTable` — which is compiled from the originals — so debugger stepping would land on the wrong lines. Making that coherent means compiling from delomboked sources too, which is a considerably bigger change and reintroduces the drift the table above rules out. I am happy to leave it exactly as it is. I would just rather it be a decision than an accident of how the config was written. One related note, since it came up while testing the above: `lombok-maven-plugin` 1.18.20.0 is genuinely the end of the line — the only newer coordinates on Central are personal republishes (`dev.pcms`, `io.github.qsy7.*`), which I would not put in an ASF build. The maintained alternative is lombok's own delombok CLI (`lombok.launch.Main`) driven by `exec-maven-plugin`; I checked its output against the plugin's and it is identical apart from the generated-at timestamp header. That is a separate cleanup, not something to fold in here. -- 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]
