ascheman commented on PR #508: URL: https://github.com/apache/maven-jar-plugin/pull/508#issuecomment-5256876269
Thanks for this migration, @desruisseaux — moving JAR creation to the `jar` `ToolProvider` with module-source-hierarchy + multi-release support (and the derived per-module POMs) is a big, elegant step. Reviewing it, I dug into the CI failures, and they share a root theme: the multi-release/module handling in `FileCollector`/`Archive` depends on the **unspecified `Files.walkFileTree` directory-iteration order** and on **jar-tool-vendor tolerances**. So it passes on macOS/Temurin and fails on Linux/Zulu — i.e. on CI. I found **six** distinct manifestations and prepared an atomic fix + tests for each. They're on `aschemaven:feat/from-archiver-to-jar-tool` (off your `5886fef`) — [compare](https://github.com/apache/maven-jar-plugin/compare/5886fef...aschemaven:feat/from-archiver-to-jar-tool). The full matrix (ubuntu/macos/windows × JDK 17/21 × rc-6) is **green** with all six, plus deterministic unit tests (`ArchiveTest`) and a `records-jar-validate` IT. Each is its own commit, so you can cherry-pick à la carte: 1. **`jar --validate` crashes on records (JDK 17/18)** — `ToolExecutor` runs `--validate` after create; the JDK 17/18 jar tool throws `This feature requires ASM8` on any record ([JDK-8282446](https://bugs.openjdk.org/browse/JDK-8282446), fixed in 19). Guard: skip validate when `Runtime.version().feature() < 19`. — `27cce4a` 2. **Base-release dir → NPE** — the base `FileSet` is seeded from whichever dir first creates the `Archive`; on some walk orders that's a `versions-modular/<n>/<module>` dir (no `module-info`), so `ModuleFinder` finds nothing and `PomDerivation` NPEs. Rebind the base to the version-less dir. — `c8a3f66` 3. **Main-Class on the wrong module** — `setMainClass` removes `Main-Class` from a manifest shared across modules, so the first module processed consumes it. Per-module manifest copy. — `18581f7` 4. **Absolute jar entry names** — `FileSet.add` relativizes only the first file; later files stay absolute and the jar tool records absolute `.class` entries ("names do not match"). Relativize all + repeat `-C` per file. — `63fe387` 5. **Empty `-C ""` for version releases** — a `versions/<n>` dir is added to its own FileSet → `relativize(dir,dir)=""`; Zulu rejects `-C dir ""`, Temurin tolerates it. Emit `.`. — `e71b7ad` 6. **Base files leak into a version FileSet** — a whole version dir is `SKIP_SUBTREE`'d, so `postVisitDirectory` (which resets to the base release) never runs; base files walked afterward land in the version FileSet as `../../../<pkg>`. Reset inline. — `0ee85a8` Because these are order/vendor-dependent they only surface on CI (Linux+Zulu); the `ArchiveTest` unit tests pin the behaviour deterministically so it can't silently regress on any walk order. Happy to open a PR against your branch if that's easier — just say the word. Thanks again, this is really nice work. -- 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]
