From 6b2b36a76dcdca21327e4e01d477613ca3471462 Mon Sep 17 00:00:00 2001 From: alexeid Date: Thu, 18 Jun 2026 11:57:12 +1200 Subject: [PATCH] Fix BEAUti startup, duplicate version listing, and example packaging Addresses issues from Remco's environment testing. BEAUti and `applauncher LogAnalyser` failed with ClassNotFoundException for beast.fx classes (BeautiConfig and the input editors). Root cause: a package's service providers are listed in its version.xml; when a version.xml scan registered a provider (via resolveLoaderFor) before the owning beast.fx plugin ModuleLayer existed, the provider was mapped to the fallback system loader, which cannot load it. registerPluginLayer uses putIfAbsent and so could not replace the stale mapping, so BEASTClassLoader.forName threw. - forName now falls through to the plugin layers on ClassNotFoundException from the mapped loader, and caches the loader that actually resolves the class (self-healing, so the slow path runs at most once per class). - resolveLoaderFor now also consults registered plugin layers, not just the boot layer. - Added a regression test reproducing the stale-mapping scenario. `beast -version` listed BEAST.base twice (once from the user package dir, once via the bundle-root version.xml that the install dir contributes). printVersion (both the beast-base and beast-fx copies) now lists each package once, noting where it was found, and reports later duplicates as skipped only in verbose mode. BEAST.base examples could not be updated by a point release: they shipped only in the bundle root, which is replaced only by a full reinstall. The BEAST.base package zip now includes examples/ (curated to match the bundle), so the package manager extracts and updates them in ~/.beast/2.8/BEAST.base/examples on install/upgrade. The three release scripts (Linux/Windows/Mac) now extract the bundle-root examples from that same package zip, giving the example set a single source of truth (beast-base/src/assembly/package.xml) so the copies cannot drift. --- beast-base/src/assembly/package.xml | 23 +++++++++- .../java/beast/base/minimal/BeastMain.java | 17 +++++-- .../java/beastfx/app/beast/BeastMain.java | 17 +++++-- .../java/beast/pkgmgmt/BEASTClassLoader.java | 31 ++++++++++++- .../ModuleLayerCrossDependencyTest.java | 44 +++++++++++++++++++ release/Linux/assemble-bundle.sh | 21 +++++---- release/Mac/build-sign-dmg.sh | 22 ++++------ release/Windows/assemble-bundle.sh | 21 +++++---- 8 files changed, 152 insertions(+), 44 deletions(-) diff --git a/beast-base/src/assembly/package.xml b/beast-base/src/assembly/package.xml index f09968ba..9e1683e1 100644 --- a/beast-base/src/assembly/package.xml +++ b/beast-base/src/assembly/package.xml @@ -4,9 +4,18 @@ + the beast.pkgmgmt module on the module path. + + Examples are shipped *inside the package* (not only in the release + bundle root) so that a BEAST.base point release can update them via the + package manager: seedBundledPackage()/the package manager extract them + to /BEAST.base/examples, which BeautiTabPane and + PackageHealthChecker discover by scanning getBeastDirectories(). The + curated subset mirrors what release/.../assemble-bundle.sh copies into + the bundle root. --> package zip @@ -20,6 +29,18 @@ + + + ${project.basedir}/src/test/resources/beast.base/examples + examples + + *.xml + spec/*.xml + nexus/** + + + + lib diff --git a/beast-base/src/main/java/beast/base/minimal/BeastMain.java b/beast-base/src/main/java/beast/base/minimal/BeastMain.java index a4cb8d3f..1598c22a 100644 --- a/beast-base/src/main/java/beast/base/minimal/BeastMain.java +++ b/beast-base/src/main/java/beast/base/minimal/BeastMain.java @@ -69,6 +69,12 @@ public static void printUsage(final Arguments arguments) { private static void printVersion() { Log.info("BEAST " + (new BEASTVersion()).getVersionString()); Log.info("---"); + // The same package can be discovered through more than one directory on + // the package path (e.g. BEAST.base is found both in the user package dir + // and via the bundle-root version.xml). List each package once, noting + // where it was found; later duplicates are reported as skipped, but only + // in verbose mode. + java.util.Set printed = new java.util.HashSet<>(); for (String jarDirName : PackageManager.getBeastDirectories()) { File versionFile = new File(jarDirName + "/version.xml"); if (versionFile.exists()) { @@ -77,9 +83,14 @@ private static void printVersion() { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); Document doc = factory.newDocumentBuilder().parse(versionFile); Element packageElement = doc.getDocumentElement(); - Log.info.print(packageElement.getAttribute("name") + " v" + packageElement.getAttribute("version")); - Log.debug.print(" " + jarDirName); - Log.info.print("\n"); + String nameAndVersion = packageElement.getAttribute("name") + " v" + packageElement.getAttribute("version"); + if (printed.add(nameAndVersion)) { + Log.info.print(nameAndVersion); + Log.debug.print(" (found in " + jarDirName + ")"); + Log.info.print("\n"); + } else { + Log.debug.println("Skipping duplicate " + nameAndVersion + " found in " + jarDirName); + } } catch (IOException| SAXException| ParserConfigurationException e) { Log.err(e.getMessage()); } diff --git a/beast-fx/src/main/java/beastfx/app/beast/BeastMain.java b/beast-fx/src/main/java/beastfx/app/beast/BeastMain.java index f87be3f8..1324fb38 100644 --- a/beast-fx/src/main/java/beastfx/app/beast/BeastMain.java +++ b/beast-fx/src/main/java/beastfx/app/beast/BeastMain.java @@ -103,6 +103,12 @@ public static void printUsage(final Arguments arguments) { private static void printVersion() { Log.info("BEAST " + (new BEASTVersion()).getVersionString()); Log.info("---"); + // The same package can be discovered through more than one directory on + // the package path (e.g. BEAST.base is found both in the user package dir + // and via the bundle-root version.xml). List each package once, noting + // where it was found; later duplicates are reported as skipped, but only + // in verbose mode. + java.util.Set printed = new java.util.HashSet<>(); for (String jarDirName : PackageManager.getBeastDirectories()) { File versionFile = new File(jarDirName + "/version.xml"); if (versionFile.exists()) { @@ -111,9 +117,14 @@ private static void printVersion() { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); Document doc = factory.newDocumentBuilder().parse(versionFile); Element packageElement = doc.getDocumentElement(); - Log.info.print(packageElement.getAttribute("name") + " v" + packageElement.getAttribute("version")); - Log.debug.print(" " + jarDirName); - Log.info.print("\n"); + String nameAndVersion = packageElement.getAttribute("name") + " v" + packageElement.getAttribute("version"); + if (printed.add(nameAndVersion)) { + Log.info.print(nameAndVersion); + Log.debug.print(" (found in " + jarDirName + ")"); + Log.info.print("\n"); + } else { + Log.debug.println("Skipping duplicate " + nameAndVersion + " found in " + jarDirName); + } } catch (IOException| SAXException| ParserConfigurationException e) { Log.err(e.getMessage()); } diff --git a/beast-pkgmgmt/src/main/java/beast/pkgmgmt/BEASTClassLoader.java b/beast-pkgmgmt/src/main/java/beast/pkgmgmt/BEASTClassLoader.java index c980753c..37a03720 100644 --- a/beast-pkgmgmt/src/main/java/beast/pkgmgmt/BEASTClassLoader.java +++ b/beast-pkgmgmt/src/main/java/beast/pkgmgmt/BEASTClassLoader.java @@ -76,6 +76,14 @@ public static Class forName(String className) throws ClassNotFoundException { ClassLoader loader = class2loaderMap.get(className); try { return Class.forName(className, false, loader); + } catch (ClassNotFoundException e) { + // The mapped loader cannot see this class. This happens when the + // provider was registered from a version.xml scan (via + // resolveLoaderFor → fallback system/context loader) before the + // owning package's plugin ModuleLayer was registered: the later + // putIfAbsent in registerPluginLayer() then cannot replace the + // stale fallback loader. Fall through to search the plugin layers + // and correct the mapping below. } catch (NoClassDefFoundError e) { String missing = e.getMessage(); if (missing != null && (missing.startsWith("beastfx/") || missing.startsWith("beastfx.") @@ -91,8 +99,15 @@ public static Class forName(String className) throws ClassNotFoundException { // 2. Plugin layers (external BEAST packages) for (ModuleLayer layer : pluginLayers) { for (Module module : layer.modules()) { + ClassLoader mcl = module.getClassLoader(); + if (mcl == null) continue; try { - return Class.forName(className, false, module.getClassLoader()); + Class c = Class.forName(className, false, mcl); + // Self-heal: cache the loader that actually resolved the + // class so subsequent lookups skip the failing mapped loader + // (and the exception-driven fall-through above). + class2loaderMap.put(className, mcl); + return c; } catch (ClassNotFoundException | NoClassDefFoundError e) { // try next module } @@ -559,6 +574,20 @@ private static ClassLoader resolveLoaderFor(String provider) { if (loader != null) return loader; } } + // Also consult already-registered plugin layers: a package's classes + // live in its own plugin ModuleLayer (e.g. beast.fx for the BEAUti + // input editors), not in the boot layer. Without this, providers listed + // in a version.xml would be mapped to the fallback system loader, which + // cannot load them, leaving forName() to fall through on every lookup. + for (ModuleLayer layer : pluginLayers) { + for (Module m : layer.modules()) { + java.lang.module.ModuleDescriptor desc = m.getDescriptor(); + if (desc != null && desc.packages().contains(pkg)) { + ClassLoader loader = m.getClassLoader(); + if (loader != null) return loader; + } + } + } return fallbackClassLoader(); } } diff --git a/beast-pkgmgmt/src/test/java/beast/pkgmgmt/ModuleLayerCrossDependencyTest.java b/beast-pkgmgmt/src/test/java/beast/pkgmgmt/ModuleLayerCrossDependencyTest.java index 8771cdcb..fa2d3449 100644 --- a/beast-pkgmgmt/src/test/java/beast/pkgmgmt/ModuleLayerCrossDependencyTest.java +++ b/beast-pkgmgmt/src/test/java/beast/pkgmgmt/ModuleLayerCrossDependencyTest.java @@ -9,12 +9,15 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.jar.JarEntry; import java.util.jar.JarOutputStream; import java.util.spi.ToolProvider; import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -64,6 +67,47 @@ public void dependentPackageResolvesAgainstAnotherPluginLayer() throws Exception assertEquals("base+plugin", result, "cross-layer call should execute"); } + /** + * Regression test for the BEAUti "ClassNotFoundException: beastfx.app.inputeditor.*" + * failure. A package's service providers are listed in its version.xml. If a + * version.xml scan registers a provider (via addServices → resolveLoaderFor) + * before the owning package's plugin ModuleLayer is registered, the provider + * is mapped to the fallback system loader, which cannot load it. registerPluginLayer() + * uses putIfAbsent and so cannot replace that stale mapping. forName() must still + * resolve the class by falling through to the plugin layers. + */ + @Test + public void forNameHealsStaleFallbackLoaderMapping() throws Exception { + ToolProvider javac = ToolProvider.findFirst("javac").orElse(null); + Assumptions.assumeTrue(javac != null, "no javac tool available - skipping"); + + Path work = Files.createTempDirectory("stale-loader-test"); + + // A modular jar exporting a provider class, mimicking beast.fx's input editors. + Path editorJar = buildModuleJar(javac, work, "fxstaletest", null, + "module fxstaletest { exports fxstaletest; }", + "fxstaletest", "Editor", + "package fxstaletest; public class Editor { }"); + + // 1. Simulate the version.xml scan that runs before the plugin layer exists: + // the provider is mapped to the fallback system loader (cannot load it). + Map> services = Map.of( + "fxstaletest.InputEditor", Set.of("fxstaletest.Editor")); + BEASTClassLoader.classLoader.addServices("FxStaleTest", services); + + // 2. Register the package's real plugin layer. putIfAbsent leaves the + // stale fallback mapping from step 1 in place. + boolean loaded = PackageManager.createAndRegisterModuleLayer( + List.of(editorJar), null, "FxStaleTest", "1.0", "FxStaleTest"); + assertTrue(loaded, "provider module should load into a plugin layer"); + + // 3. Despite the stale mapping, forName must resolve the class via the + // plugin layer (previously threw ClassNotFoundException here). + Class editor = BEASTClassLoader.forName("fxstaletest.Editor"); + assertNotNull(editor); + assertEquals("fxstaletest.Editor", editor.getName()); + } + /** Compile a single-package module and pack it into a jar; return the jar path. */ private static Path buildModuleJar(ToolProvider javac, Path work, String moduleName, Path modulePathJar, String moduleInfo, diff --git a/release/Linux/assemble-bundle.sh b/release/Linux/assemble-bundle.sh index eeb14426..516bba52 100755 --- a/release/Linux/assemble-bundle.sh +++ b/release/Linux/assemble-bundle.sh @@ -152,20 +152,19 @@ else echo " WARNING: DensiTree.jar not found — densitree script will not work" fi -# ── Step 6: Copy examples ───────────────────────────────────────────────────── +# ── Step 6: Extract examples from the BEAST.base package zip ────────────────── +# The bundle-root examples/ are extracted from the same package zip seeded into +# the user dir (lib/packages/), so the example set has a single source of truth +# (beast-base/src/assembly/package.xml) and the bundle-root and packaged copies +# cannot drift. The bundle-root copy is kept so `beast -validate examples/...` +# works from the install dir before first-run seeding. echo "" -echo "==> Step 6: Copying examples..." -EXAMPLES_DIR="$REPO_ROOT/beast-base/src/test/resources/beast.base/examples" -if [ -d "$EXAMPLES_DIR" ]; then - find "$EXAMPLES_DIR" -maxdepth 1 -name "*.xml" -exec cp {} "$BUNDLE/examples/" \; - mkdir -p "$BUNDLE/examples/spec" - [ -d "$EXAMPLES_DIR/spec" ] && \ - find "$EXAMPLES_DIR/spec" -maxdepth 1 -name "*.xml" -exec cp {} "$BUNDLE/examples/spec/" \; - [ -d "$EXAMPLES_DIR/nexus" ] && cp -r "$EXAMPLES_DIR/nexus" "$BUNDLE/examples/" +echo "==> Step 6: Extracting examples from $(basename "$BASE_PKG_ZIP")..." +if unzip -o -q "$BASE_PKG_ZIP" 'examples/*' -d "$BUNDLE"; then EXAMPLE_COUNT=$(find "$BUNDLE/examples" -name "*.xml" | wc -l | tr -d ' ') - echo " Copied ${EXAMPLE_COUNT} example XML files (incl. spec/)" + echo " Extracted ${EXAMPLE_COUNT} example XML files (incl. spec/)" else - echo " WARNING: examples not found at $EXAMPLES_DIR" + echo " WARNING: no examples found in $BASE_PKG_ZIP" fi # ── Step 7: Copy version.xml and docs ──────────────────────────────────────── diff --git a/release/Mac/build-sign-dmg.sh b/release/Mac/build-sign-dmg.sh index 633a21ae..f8384f87 100755 --- a/release/Mac/build-sign-dmg.sh +++ b/release/Mac/build-sign-dmg.sh @@ -558,21 +558,15 @@ else fi # ── examples/ — example BEAST XML files ────────────────────────────────────── -EXAMPLES_DIR="$REPO_ROOT/beast-base/src/test/resources/beast.base/examples" -if [ -d "$EXAMPLES_DIR" ]; then - echo " Copying examples/..." - mkdir -p "$OUTPUT/examples" - # Copy top-level XML files - find "$EXAMPLES_DIR" -maxdepth 1 -name "*.xml" -exec cp {} "$OUTPUT/examples/" \; - # Copy nexus subdirectory - if [ -d "$EXAMPLES_DIR/nexus" ]; then - cp -r "$EXAMPLES_DIR/nexus" "$OUTPUT/examples/nexus" - fi - if [ -d "$EXAMPLES_DIR/spec" ]; then - cp -r "$EXAMPLES_DIR/spec" "$OUTPUT/examples/spec" - fi +# Extracted from the BEAST.base package zip seeded into the user dir +# (Contents/app/packages/), so the example set has a single source of truth +# (beast-base/src/assembly/package.xml) and the bundle-root and packaged copies +# cannot drift. The bundle-root copy is kept so `beast -validate examples/...` +# works from the install dir before first-run seeding. +if [ -n "$BASE_PKG_ZIP" ] && unzip -o -q "$BASE_PKG_ZIP" 'examples/*' -d "$OUTPUT"; then + echo " Extracted examples/ from $(basename "$BASE_PKG_ZIP")" else - echo " WARNING: $EXAMPLES_DIR not found — skipping examples/" + echo " WARNING: no examples found in BEAST.base package zip — skipping examples/" fi # ── README and LICENSE ───────────────────────────────────────────────────── diff --git a/release/Windows/assemble-bundle.sh b/release/Windows/assemble-bundle.sh index 8e4647ab..3a7592bb 100644 --- a/release/Windows/assemble-bundle.sh +++ b/release/Windows/assemble-bundle.sh @@ -244,20 +244,19 @@ WINBIN_DIR="$SCRIPT_DIR/bat" mkdir -p "$BUNDLE/bat" cp "$WINBIN_DIR/"*.bat "$BUNDLE/bat/" -# ── Step 7: Copy examples ───────────────────────────────────────────────────── +# ── Step 7: Extract examples from the BEAST.base package zip ────────────────── +# The bundle-root examples/ are extracted from the same package zip seeded into +# the user dir (app/packages/), so the example set has a single source of truth +# (beast-base/src/assembly/package.xml) and the bundle-root and packaged copies +# cannot drift. The bundle-root copy is kept so `beast -validate examples/...` +# works from the install dir before first-run seeding. echo "" -echo "==> Step 7: Copying examples..." -EXAMPLES_DIR="$REPO_ROOT/beast-base/src/test/resources/beast.base/examples" -if [ -d "$EXAMPLES_DIR" ]; then - mkdir -p "$BUNDLE/examples/spec" - find "$EXAMPLES_DIR" -maxdepth 1 -name "*.xml" -exec cp {} "$BUNDLE/examples/" \; - [ -d "$EXAMPLES_DIR/spec" ] && \ - find "$EXAMPLES_DIR/spec" -maxdepth 1 -name "*.xml" -exec cp {} "$BUNDLE/examples/spec/" \; - [ -d "$EXAMPLES_DIR/nexus" ] && cp -r "$EXAMPLES_DIR/nexus" "$BUNDLE/examples/" +echo "==> Step 7: Extracting examples from $(basename "$BASE_PKG_ZIP")..." +if unzip -o -q "$BASE_PKG_ZIP" 'examples/*' -d "$BUNDLE"; then EXAMPLE_COUNT=$(find "$BUNDLE/examples" -name "*.xml" | wc -l | tr -d ' ') - echo " Copied ${EXAMPLE_COUNT} example XML files (incl. spec/)" + echo " Extracted ${EXAMPLE_COUNT} example XML files (incl. spec/)" else - echo " WARNING: examples not found at $EXAMPLES_DIR" + echo " WARNING: no examples found in $BASE_PKG_ZIP" fi # ── Step 8: Copy version.xml and docs ────────────────────────────────────────