jlink 4.0.2 + native installers + namegen cleanup#7568
Merged
Conversation
- NameList/RuleSet: precompute a cumulative-weight array at construction so pick() is O(log n) via binarySearch instead of two linear passes. Zero-weight entries are filtered into a parallel positives array, which keeps binarySearch correct when duplicates would otherwise let it land on a zero-weight index. Both records become final classes since cached state can't live on a record. - DataValue: drop the hand-rolled DataSubValue linked list in favour of a lazily-allocated LinkedHashMap. putIfAbsent preserves the first-write-wins semantics the old recursive get() relied on. - NameGenLazyData.gendersForRuleset: read directly from the ruleset's RuleSetMeta.categoryTitles() instead of scanning every Sex: bucket. - NameGenerator.assemble: defer meaning/pronunciation StringBuilder allocation until a part actually overrides one, seeding from the name built so far. Avoids two builders' worth of churn on the common path where no part has sub-values.
The jlink plugin's `additive = true` mode emits both the auto-derived and the user-listed `requires` blocks without dedup. asm 9.10 (pulled in by jlink 4.0.1) parses Java 25 class files that 9.9.1 silently skipped, growing the auto-derived set enough to collide with our explicit list and breaking `createMergedModule` with duplicate-requires errors. Bump to 4.0.2 and keep only the modules the scanner can't infer on its own.
Brief description of each overload's contract (direct Element children, optional tag-name filter) so future readers don't have to re-derive it from the stream pipeline.
Vest
changed the title
Drop the Os.FAMILY_MAC || Os.FAMILY_UNIX predicate that excluded Windows from fullJpackage. The carve-out dates from when Windows shipped via NSIS (`buildNsis`); that path was retired in be48763 but the release-side hookup to invoke jpackage on Windows was never added, leaving Windows release artifacts as the runtime zip only.
fullJpackage was wired to assembleJpackageImage (Mac: patchMacJpackage), not the jpackage installer task — so each release shipped only the jlinkZip runtime image, never a .dmg/.deb/.exe. Make jpackage depend on the data-bundling step so it packages an image that already contains data/plugins/preview/outputsheets, then point fullJpackage at jpackage. Pin one installerType per OS (dmg / deb / exe) — without it jpackage emits both formats per platform, doubling build time and (on Linux) requiring rpmbuild that isn't installed on the Ubuntu runner. Add --win-shortcut / --win-menu so the .exe creates Start-menu and desktop entries; the existing --linux-shortcut and Mac-specific opts were already in place. WiX 3.14 needed for the .exe is preinstalled on the windows-latest runner.
Vest
changed the title
Same self-contained app the installer ships, but as an unzip-and-run archive — useful for users who don't want to run an installer (USB sticks, locked-down environments, etc.). Per-platform: pcgen-<version>-<os>-<arch>-portable.zip. The zip captures whatever fullJpackage left in build/jpackage/PcGen[.app], so it includes data/plugins/preview/outputsheets and (on Mac) the patched Info.plist + MacDirLauncher.
Contributor
Author
|
I am optimizing a CI build again - I want to build artifacts for different platforms. When i am ready I will create a new pr |
jpackage names Debian packages pcgen_<version>_<arch>.deb (underscore), which the existing pcgen-*.* glob (hyphen) misses. Linux release runs were uploading everything except the .deb. Add a second include for the underscore form. .rpm uses pcgen-<version>.<arch>.rpm so it was already covered.
3 tasks
The image-*.zip artifacts (output of jlinkZip) are a JVM runtime image, not a runnable PcGen — no launcher, no /data, /plugins, /preview, or /outputsheets. Users who tried to run them got the "missing required folders" dialog. Now that fullJpackage produces real installers and a portable zip with the same .app the installer ships, image-*.zip is just a confusing leftover. Drop it from the release upload (autobuild keeps it via buildDist). The SHA256-digests-*.txt files are also redundant: GitHub computes a SHA-256 for every release asset automatically and exposes it via the API. Anyone verifying integrity can shasum the file locally.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three independent threads bundled into one PR — they were uncovered while debugging the first one. OK to squash on merge: the commit history was useful while iterating but the end-state changes are small and cohesive.
1.
jlinkplugin upgrade and merged-module fixorg.beryx.jlink4.0.1 → 4.0.2.createMergedModulefailed with duplicate-requirescompile errors. Root cause: asm 9.10 (pulled in by jlink 4.0.1) parses Java 25 class files that 9.9.1 silently skipped, so the plugin's auto-derivedrequiresset grew enough to collide with our explicit list underadditive = true.java.naming,java.scripting,java.management,jdk.httpserver,jdk.unsupported.desktop).2. Release pipeline now ships real native installers + portable zips
fullJpackage. A leftoverOs.FAMILY_MAC || Os.FAMILY_UNIXpredicate inrelease.gradledated from when Windows shipped via NSIS (buildNsis); NSIS was retired inbe48763530but the jpackage hookup was never added on Windows. Removed the predicate.fullJpackageonly built the runtime image, never the installer. It depended onassembleJpackageImage(orpatchMacJpackageon Mac), not thejpackagetask that produces.dmg/.deb/.exe. Releases shipped onlyimage-<os>.zipfromjlinkZip— a JVM runtime without/data,/plugins,/preview,/outputsheets, so launching it gave the "missing required folders" dialog. Madejpackagedepend on the data-bundling step (and on Mac, onpatchMacJpackageso the patched plist +MacDirLaunchergo into the installer); pointedfullJpackageatjpackage.installerTypeper OS (dmg/deb/exe) — without it jpackage emits both formats per platform, doubling build time and (on Linux) requiringrpmbuildthat isn't installed on the Ubuntu runner.--win-shortcut --win-menuso the.execreates Start-menu and desktop entries.portableZiptask producespcgen-<version>-<os>-<arch>-portable.zipalongside the installer — same self-contained app, but unzip-and-run for users who don't want an installer (USB sticks, locked-down environments, etc.). Wired intoassembleArtifactsso CI uploads it to the GitHub Release..debnot being uploaded.jpackagenames Debian packagespcgen_<version>_<arch>.deb(underscore), which the existingpcgen-*.*glob (hyphen) missed. Addedpcgen_*.debto the include set..rpm(pcgen-<version>.<arch>.rpm) was already covered.image-<os>.zipfrom the release uploads. Output ofjlinkZip— a JVM runtime image without/data,/plugins,/preview, or/outputsheets. Misleadingly named: users who tried to run it got the "missing required folders" dialog. Autobuild keeps it viabuildDist(separate distribution channel, unchanged).checksumtask andSHA256-digests-*.txtfiles. GitHub Releases compute and expose a SHA-256 for every uploaded asset automatically; anyone verifying integrity canshasum -a 256against the value GitHub provides. The 4 generated digest files were noise.3.
pcgen.core.namegenperf / cleanup (no behavior change)NameList/RuleSet: O(n) two-pass weightedpick()→ O(log n)binarySearchover a precomputed cumulative-weight array.DataValue: hand-rolled linked list (DataSubValue) → lazyLinkedHashMap.NameGenLazyData.gendersForRuleset: O(categories) bucket scan → O(1) lookup viaRuleSetMeta.categoryTitles().NameGenerator.assemble: lazy meaning / pronunciationStringBuilders; only allocate when a part actually overrides.Final release-asset list (per platform)
After this PR, each release produces a focused, user-meaningful asset list:
pcgen-<v>.dmgpcgen-<v>-mac-aarch64-portable.zippcgen_<v>_amd64.debpcgen-<v>-linux-x64-portable.zippcgen_<v>_arm64.debpcgen-<v>-linux-aarch64-portable.zippcgen-<v>.exepcgen-<v>-windows-x64-portable.zipPlus
pcgen-<v>-sources.jar. 9 user-meaningful assets, down from 15 mixed-purpose ones. Per-asset SHA-256 hashes are available on the GitHub Release UI/API.Test plan
./gradlew createMergedModulesucceeds (was the original failure)../gradlew fullJpackageon Mac producespcgen-6.09.08.dmg(160 MB) with all four required folders insidePcGen.app/Contents/app/../gradlew portableZipon Mac producespcgen-6.09.08.RC1-mac-aarch64-portable.zip(148 MB) with the same self-containedPcGen.app.Vest/pcgen(run26779067448, tagv6.09.08.RC1): macOS, Ubuntu x64, Ubuntu ARM, and Windows jobs all green; produced.dmg+.exe+ portable zips on all OSes. (Note: that run predates the.debglob fix and theimage-*.zip/ digest-file removal — a fresh run is recommended before merge to confirm the final asset list.)./gradlew :test --tests 'pcgen.core.namegen.*' --tests 'pcgen.gui3.namegen.*'(173 tests, 0 failures).