Remove commons-io as a direct dependency#5
Open
Vest wants to merge 20 commits into
Open
Conversation
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit 6217b6e. |
Vest
force-pushed
the
jpms_subprojects
branch
2 times, most recently
from
May 19, 2026 05:32
9ddf261 to
2cb73bb
Compare
…mula repositories (PCGen#7563) * Convert PCGen-base and PCGen-Formula to JPMS subprojects Restructure the build to make PCGen-base and PCGen-Formula proper Java modules (JPMS) as Gradle subprojects, replacing the old external JAR dependencies (net.sourceforge.pcgen:PCGen-base:1.0.170 and net.sourceforge.pcgen:PCGen-Formula:1.0.200). Build infrastructure: - Add 'PCGen-base' and 'PCGen-Formula' to settings.gradle as included projects - Write new build.gradle for each subproject (java-library, Java 25, JUnit 5) - Replace external JAR deps with project(':PCGen-base') and project(':PCGen-Formula') - Remove ivy fileRepo repository (no longer needed) - Remove PCGen-base/PCGen-Formula from jlink forceMerge (proper named modules now) - Move JavaCompile/Test task config out of allprojects{} to root-only scope Module descriptors: - Create module-info.java for pcgen.base (exports utility packages) - Create module-info.java for pcgen.formula (requires transitive pcgen.base, exports formula parser and solver packages) - Update main module-info.java: requires pcgen.base; requires pcgen.formula Split-package resolution (JPMS forbids two modules owning same package): - Move generic formula classes to PCGen-Formula: AddingFormula, DividingFormula, MultiplyingFormula, SubtractingFormula, ReferenceFormula - Move generic format/test classes to PCGen-base: Dice, Die, DiceFormat, InequalityTester - Delete TypeSafeConstant from main (identical copy exists in PCGen-base) - Relocate pcgen.base.formula.Formula to pcgen.cdom.formula.Formula (legacy interface that depends on pcgen.core — cannot live in generic module) - Relocate pcgen.base.calculation.* to pcgen.cdom.calculation.* (PCGen-specific adapter layer, not part of the generic solver) Pre-commit JavaCC parser: - Generate parser source from formula.jjt using jjtree+javacc 7.0.13 - Place all generated files in PCGen-Formula/code/src/java/pcgen/base/formula/parse/ - No build-time generation needed Update ~150 files for import changes across main source and tests. Both subprojects compile successfully. Main source has remaining compilation errors from API differences between the old published JARs and the current in-repo source (Phase 6: API migration still needed). * Phase 6: Migrate main source to new PCGen-Formula/PCGen-base APIs Complete API migration enabling the main module to compile against the in-repo PCGen-Formula and PCGen-base subprojects (current source) rather than the old published JARs. Key API changes applied: - LegalScope -> ImplementedScope (getParentScope -> drawsFrom/isGlobal) - FormulaManager removed; replaced by ManagerFactory pattern - ScopeManagerInst -> ImplementedScopeLibrary (registerScope -> addScope) - VariableStrategy -> DependencyStrategy - Modifier.getDependencies -> captureDependencies; added isValid - NEPFormula.getDependencies -> captureDependencies - ScopeInstance.getLegalScope -> getImplementedScope - VarScoped: removed getLocalScopeName/getVariableParent from interface, added getProviderFor(ImplementedScope) for scope hierarchy navigation - SolverManager: addModifierAndSolve -> addModifier, solveChildren -> processSolver, getDefaultValue -> getDefault - FormatManagerFactory.build and FormatManagerLibrary.getFormatManager signature changes (Optional parameters) - VariableLibrary.getVariableFormat now returns Optional<FormatManager<?>> - ScopeInstanceFactory.get(String, VarScoped) replaces old Optional-based API - FormulaFactory.getValidFormula no longer takes FormulaManager parameter Structural additions: - GlobalPCVarScoped: sentinel VarScoped for PC global scope instances - PCGenScoped.getLocalScopeName(): moved from VarScoped interface to PCGen-specific PCGenScoped interface - ImplementedScopeLibrary.getScopes(): collection accessor for GUI code - VariableContext.validateDefaults(): delegates to SupplierValueStore * Fix simple test compilation errors for new PCGen-Formula/PCGen-base APIs Mechanical updates to test/testcommon sources to align with renamed and reshaped APIs in the JPMS subprojects: - Modifier.getDependencies → captureDependencies, added isValid stub - NEPFormula.getDependencies → captureDependencies - LegalScope → ImplementedScope / PCGenScope type references - SimpleScopeInstance constructor: removed Optional parent parameter - AggressiveSolverManager/DynamicSolverManager → SimpleSolverManager - ColumnFormatFactory/TableFormatFactory.build() now takes Optional params Remaining test errors require rewriting AbstractFormulaTestCase (removes FormulaManager) and dependent test classes — left for a separate change. * Fix test compilation and runtime failures for JPMS API migration - Add equals/hashCode to GlobalPCVarScoped to fix ScopeInstance identity mismatches when multiple instances are created across test and production code - Fix scope getName() methods to return fully-qualified names (e.g. "PC.SKILL") matching the registration keys used by ImplementedScopeLibrary - Add getFunctionLibrary() and getOperatorLibrary() accessors to VariableContext for test infrastructure access - Rewrite AbstractFormulaTestCase to use ManagerFactory/ScopeInstanceFactory instead of removed FormulaManager - Update SimpleSolverManager.processSolver to preserve existing store values when no solver is built, preventing VariableChannel.set() from being overwritten by format defaults - Adapt tests to explicitly call processSolver after addModifier since SimpleSolverManager does not auto-evaluate like the old DynamicSolverManager * Update Gradle wrapper to 9.5.1 * Remove processSolver calls after user-input channel writes SimpleSolverManager.processSolver() writes the format default when no solver exists, which overwrites values set via VariableChannel.set() and ChannelUtilities.setGlobalChannel(). This was correct for the old DynamicSolverManager (which auto-cascaded), but incorrect for SimpleSolverManager where channel writes are the final value. - Revert SimpleSolverManager.processSolver() to original upstream logic - Remove processSolver() call from VariableChannel.set() - Remove processSolver() call from ChannelUtilities.setGlobalChannel() - Remove unused SolverManagerFacet field from ChannelUtilities * Open pcgen.cdom.base to pcgen.base for StagingProxy reflection access PCGen-base's StagingProxy.applyTo() reflectively invokes methods on VarHolder (in pcgen.cdom.base). With proper module boundaries enforced, this requires an explicit opens directive. * Add JUnit Platform Launcher to subproject test dependencies JPMS modular projects with inferModulePath require the launcher artifact explicitly on the test runtime classpath. * Revert String equality branch in CaseInsensitiveString.equals() Reverts the change from a7e1952 which added an `instanceof String` branch to equals(). This violates the equals symmetry contract: cis.equals("Foo") returned true but "Foo".equals(cis) always returns false since String is unaware of CaseInsensitiveString. The upstream pcgen-base repo never accepted this change, and the existing CaseInsensitiveStringTest.testString() explicitly asserts that this comparison must be false. Callers that need case-insensitive comparison with a raw String should wrap it in a CaseInsensitiveString first. * Restore processSolver after addModifier/removeModifier in SolverManagerFacet The Phase 6 API migration (094525d) replaced addModifierAndSolve() with addModifier() but omitted the follow-up processSolver() call that recomputes the variable value. Without it, modifiers were registered but never evaluated, causing GlobalModifyTest to hang waiting for a value that was never computed. Also short-circuit global-scope resolution: when the target scope is global, call scopeFacet.getGlobalScope() directly instead of walking the VarScoped hierarchy — global variables have no meaningful scoped object to traverse. In GlobalModifyTest.targetFacetCount(), guard against an empty diagnose list (which is valid when no solver has been built yet) and remove a stale comment about a missing API method. * Resolve global scope correctly in SolverManagerFacet When MODIFY is parsed inside a non-global scope (e.g., MODIFY:Face on a SIZE object), VarModifier.getLegalScope() reports the parsing scope, not the variable's actual scope. Following that path produced a ScopeInstance that did not match the one used when reading the variable, so modifiers were applied to a different VariableID than channels read from. Routine the scope through resolveScope(): if the parsed scope is global, or if the variable is legally defined at the global scope, use the global ScopeInstance. Otherwise fall back to the local scope as before. Fixes the Pathfinder FACE tests (which use MODIFY:Face on SIZE objects to set the global Face variable to "5,5") and the GlobalModifyTest identity-mismatch failures. * Copy globalVarScopedMap entry on PlayerCharacter clone PlayerCharacter.clone() invokes bean.copyContents(id, aClone.id) on every facet, but AbstractItemFacet.copyContents() copies the underlying map directly without going through set(), so ScopeFacet.set()'s side effect of registering the GlobalPCVarScoped sentinel was skipped on the clone. The cloned character then had a null entry in globalVarScopedMap, and getGlobalScope() handed back a ScopeInstance whose owner was null, causing NullPointerExceptions in VarScoped.getProviderFor() during the 26 pcGenGUI*Test save/restore flows. Override copyContents() to also copy the globalVarScopedMap entry. * Fix CI release pipeline after JPMS subproject migration - jlink: add `addExtraDependencies('javafx')` so the merged module (which inherits `requires javafx.graphics` from controlsfx) can resolve JavaFX modules from `mods/lib` during merged-module compilation. - jlink: declare `prepareMergedJarsDir.dependsOn copyToLibs` to satisfy Gradle 9.5's implicit-dependency validation; both tasks read/write `build/libs/`. - Test config: move the JavaFX `--module-path mods/lib --add-modules javafx.*` JVM args out of `allprojects { tasks.withType(Test) }`. The pure-formula subprojects (PCGen-base, PCGen-Formula) have no `mods/lib` and don't depend on JavaFX, so applying those args to their tests crashed JVM startup with `FindException: Module javafx.web not found`. - Remove unused imports flagged by checkstyle after the JPMS API migration. * Generate PCGen-Formula parser at build time via JJTree+JavaCC The PCGen-Formula parse package gitignores seven JavaCC-generated files (FormulaParser, FormulaParserConstants, FormulaParserTokenManager, ParseException, Token, TokenMgrError, SimpleCharStream). They were previously produced by an Ant build that no longer runs after the JPMS subproject migration, so clean CI checkouts (including CodeQL's autobuild) failed compileJava with "cannot find symbol: FormulaParser". Add a javacc configuration and two JavaExec tasks (jjtree, javacc) to PCGen-Formula/build.gradle. JJTree converts formula.jjt into an annotated .jj under build/generated/jjtree, then JavaCC emits the seven parser/token classes into build/generated/sources/javacc, which is added as a secondary srcDir of the main source set. The hand-maintained AST/Visitor/Node/SimpleNode/Operator classes in code/src/java remain canonical because JJTree's auxiliary outputs land in a directory not on the source path. * Generate redundant JJTree outputs at build time The 16 AST classes plus Node, JJTFormulaParserState, FormulaParserTreeConstants, FormulaParserVisitor, and FormulaParserDefaultVisitor in PCGen-Formula/code/src/java/pcgen/base/formula/parse were byte-identical to JJTree's output from formula.jjt — pure boilerplate that didn't need to live in source control. Wire JJTree's output dir as a secondary srcDir of the main source set, drop JJTree's SimpleNode.java stub in a doLast (the hand-edited SimpleNode in code/src/java is canonical), and remove the 21 redundant files. The .gitignore in the parse package now covers all generated artifacts so they can't accidentally be re-added. Operator.java and SimpleNode.java are the only remaining hand-maintained files in that package. * Document why SimpleNode is hand-maintained Note the PCGen-specific Operator/text fields, the ~50 call sites that prevent a rename, and the NODE_CLASS=PCGenBaseNode alternative for any future cleanup. * Fix DynamicScope.getName() to return fully-qualified scope name ImplementedScopeLibrary (new API) keys scopes by scope.getName() directly, whereas the old ScopeManagerInst built the full dotted name from the scope hierarchy. All hardcoded PCGenScope implementations (SkillScope, StatScope, etc.) were updated to return their full name (e.g. "PC.SKILL"), but DynamicScope was missed — it still returned only the category's local name (e.g. "MOVEMENT" instead of "PC.MOVEMENT"). This caused MODIFYOTHER:PC.MOVEMENT|... and LOCAL:PC.MOVEMENT|... tokens to fail with "illegal variable scope" warnings at load time, breaking DataLoadTest for any source that uses dynamic scopes (e.g. Starfinder armor entries using MODIFYOTHER to apply movement penalties). * Fix getLocalVariableID to use owner's local scope, not global PC scope In the Phase 6 API migration, the call: instFactory.get(localScopeName.get(), Optional.of(owner)) was replaced with: SCOPE_FACET.get(id, owner) ScopeFacet.get(CharID, VarScoped) always uses GlobalPCScope.GLOBAL_SCOPE_NAME ("PC"), so variables declared in child scopes (e.g. CHANNEL*STATSCORE in PC.STAT) could not be found — VariableManager.getActiveScope only walks drawsFrom() upward, not into child scopes. Restore the original intent: get the owner's proper local scope name via PCGenScoped.getLocalScopeName() and use the ScopeInstanceFactory directly. * Address Sonar issues in VariableUtilities Replace wildcard VariableID<?> returns on getLocalVariableID with a generic <T> parameter (S1452), matching the existing pattern on getGlobalVariableID. Replace the unguarded Optional.get() on the owner's local scope name with orElseThrow (S3655), carrying the variable name, owner class, and owner identity in the message so a misrouted call can be diagnosed from the log alone. * Fix stale links and git rebase command in README Replace defunct AdoptOpenJDK links with Eclipse Temurin (Adoptium). Correct git rebase command: fetch before checkout, and rebase from upstream/master. * Include PCGen-base and PCGen-Formula test results in CI report After the JPMS subproject migration, subprojects produce their own test results under PCGen-base/ and PCGen-Formula/. Add those paths to the Publish Test Results step so they are not silently omitted. * Drop unused orange-extensions compileOnly dependency com.yuvimasory:orange-extensions had no references in any source file. The library exposes Apple-specific eawt APIs that the project does not use; the dependency was carried by the build configuration alone. * Move xmlunit-core to testImplementation scope Only PcgenFtlTestCase referenced org.xmlunit.*; no production code did. Keeping it on the production module path forced a `requires org.xmlunit;` in module-info and pulled the jar into the runtime image for nothing. * Add UNICODE_INPUT=true to suppress JavaCC non-ASCII warning The BASIC_LETTER token definition covers Unicode ranges beyond Latin. Without UNICODE_INPUT=true, JavaCC warns about non-ASCII characters in the generated regex. The option also ensures the parser handles Unicode input correctly when a non-default Reader is used. * Document fullJpackage as the correct native bundle task jpackageImage alone skips assembleJpackageImage, leaving data/plugins/ preview/outputsheets out of the bundle. fullJpackage runs the full chain. Also document the macOS .DS_Store workaround. * Remove dead cross-platform build infrastructure from jlink/jpackage Pre-late-2024, PCGen used NSIS for the Windows installer. NSIS is script-driven and platform-agnostic, so a single host could produce installers for all five target platforms (linux x64/aarch64, mac x64/aarch64, windows x64). The all-platforms JDK download chain (downloadJDKs / extractJDKs / downloadJavaFXMods, plus the five-entry targetPlatform() block in jlink {}) was load-bearing for that workflow, and PCGEN_ALL_PLATFORMS=true was the switch that turned it on. NSIS was retired in commits 306cee4..be48763 in favor of jpackage. jpackage is not platform-agnostic: it shells out to native OS tooling (hdiutil/pkgbuild on macOS, WiX on Windows, dpkg-deb on Linux), and the Beryx jlink plugin's mergedModule step invokes the target JDK's native javac to synthesize a module-info. The moment NSIS left, cross-platform builds from a single host stopped working - verified 2026-05-19 on macOS aarch64, where PCGEN_ALL_PLATFORMS=true ./gradlew jlink fails with "jdk_linux_aarch64/bin/javac: cannot execute binary file". CI sidestepped this by moving to a per-OS matrix (.github/workflows/gradle-release.yml) where each runner builds only its own platform. PCGEN_ALL_PLATFORMS is never set in any workflow and would fail on every GitHub-hosted runner if it were. This commit removes the now-dead surface: - The PCGEN_ALL_PLATFORMS env-var branches in jlink {} and tasks.named("jlink"); registers only the host targetPlatform(). - Aggregator tasks downloadJDKs, extractJDKs, downloadJavaFXMods. - The unused 'jre' task (no callers). Per-platform downloadJdk_*, extractJdk_*, downloadJfxMods_*, and extractJfxMods_* tasks are kept - they back the host-only path and CI caches them by glob. AGENTS.md updated to point at the per-platform task names. * Collapse per-platform JDK/JFX tasks to single host-targeted tasks Follow-up to "Remove dead cross-platform build infrastructure". The prior commit removed the all-platforms aggregator tasks and the five-entry targetPlatform() block, but the four platforms.each { } loops were left intact, registering 20 download/extract tasks of which only the host platform's 4 ever fire. Collapse the loops to single tasks: downloadJdk, extractJdk, downloadJfxMods, extractJfxMods. Filenames remain platform-stamped (jdks/jdk_${hostOs}_${hostArch}.${ext}) so the GitHub Actions cache glob in gradle-release.yml keeps working - per-runner cache keys already isolate the linux/mac/windows builds. Remove the platforms ext list, which existed only to drive the loops. Lift host-OS/arch detection to a single computation at script scope (above jlink {}) and reuse it from jlink {}, jpackage {}, and tasks.named("jlink") instead of recomputing in three places. Verified by running ./gradlew jlink end-to-end on macOS aarch64: build/image/pcgen-mac-aarch64/bin/java reports openjdk 25.0.3. Updates CI workflow comments and AGENTS.md to reference the new flat task names. * Disable unused distZip/distTar tasks The application plugin's distZip/distTar produce build/distributions/ archives containing the pcgen jar plus ~150 runtime dependency jars and the generated start scripts. Nothing in this project consumes them: - CI publishes the custom 5-zip layout from buildDist (data, docs, program, libs, image) plus jpackage native installers. The release pipeline never reads build/distributions/. - End users install via jpackage-produced .dmg/.pkg/.exe/.deb/.rpm with a bundled JRE, not a raw jar pile that needs a system JDK. - Local dev uses ./gradlew run / qbuild, which don't touch dist*. Disable both with enabled = false. They stay in the task graph (assemble depends on them) but skip their actions, so every build stops materializing ~150 jars into build/distributions/ for nobody to read. Drop distTar/distZip from sourcesJar's dependsOn — that was a stale workaround for a Gradle implicit-dependency warning between copyToLibs and the dist tasks, now moot. sourcesJar's content comes from sourceSets.main.allSource, which is independent of dist*. * Remove dead build tasks and stale debug prints - buildonly: registered in 2014, never referenced anywhere in the repo (no CI, docs, scripts). Functionally a worse qbuild — same dependency on copyToOutput but missing the actual copy logic that makes qbuild useful. Pure dead weight. - quickbuild: same story, never referenced. "build runnable output and run tests" is just `./gradlew qbuild test` — no dedicated task needed. - println("IN copyToOutput") / println("IN buildonly"): debug prints left over from the 2014 Gradle conversion (commit f00f99c). They spammed stdout on every build with no useful info. - Stale TODO + commented-out `dependsOn copyToOutput` in the build task: documenting a non-change from 18 months ago. The decision to not have build populate output/ is permanent — qbuild fills that role for devs who want it. Kept mustRunAfter clean (load-bearing) with a clearer comment. - Dropped the now-orphan "// Alias tasks" comment.
Step 1 of removing the commons-io dependency. Adds private static helpers in BatchExporter for getExtension/removeExtension semantics (last-dot-in-final-segment), and uses java.io.File.getName() with a null guard in SpellsKnownTab.
Step 2 of removing commons-io. The behavior is preserved: UTF-8 encoding, System.lineSeparator() between lines, and a trailing separator after the last line — matching FileUtils.writeLines.
Covers the structural contract of the party file: two lines, version prefix, comma-separated character paths, trailing line separator, UTF-8 encoding. Verified to pass against both the old FileUtils.writeLines implementation and the new Files.writeString one.
Signed-off-by: Vest <Vest@users.noreply.github.com>
Step 3 of removing commons-io. Adds a small private helper that mirrors FileUtils.listFiles(File, String[], boolean) semantics: recursive, regular files only, case-sensitive extension match.
The original code wrapped a ByteArrayOutputStream in a TeeOutputStream so the export simultaneously fed an in-memory buffer (consumed by FOP) and an optional debug temp file. Since the bytes are fully materialised in memory before FOP runs, the tee was unnecessary: write the temp file sequentially after the export completes instead. This removes the last commons-io dependency in this file and lets us delete the internal TeeOutputStream helper.
exportCharacterToPDF inspects task.getErrorMessages() after running FOP and returns false (with a logged message) when FOP reports problems. exportPartyToPDF was missing the same check, so a party PDF that FOP failed to render would silently return true and leave the user thinking the export succeeded. Mirror the character path: log and return false when FOP produces a non-blank error message.
The previous implementation called File.list(FilenameFilter) and used
the filter lambda for its side effect — deleting matching files inside
the predicate and always returning false so list() would return an
empty array the caller then ignored. That misuse hid three real
problems:
- tf.delete() returned false silently; failures to delete were
invisible.
- The defensive try/catch around the filter caught exceptions a
FilenameFilter is not allowed to throw — pure cargo cult.
- File.list returns null when the directory does not exist; the
accidental side-effect form happened to tolerate that, but only
by coincidence.
Use listFiles to actually list the temp files matching the prefix,
guard against the null result, then iterate and delete with a logged
warning when delete fails. Drop the manual File.separator concatenation
in favour of SettingsHandler.getTempPath() (already a File).
exportCharacterToNonPDF already used StandardCharsets.UTF_8 but four sibling methods (exportPartyToNonPDF, both exportParty overloads, and the private exportCharacter overload) still passed the literal string "UTF-8" to OutputStreamWriter. The two forms behave identically but the string form forces callers to deal with UnsupportedEncodingException and lets a typo go undetected until runtime. Switch all four sites to the constant so the file is internally consistent and the encoding is checked at compile time.
removeTemporaryFiles previously used File.delete, which returns a bare boolean on failure with no indication of why the OS refused the operation — permission denied, file locked by another process, filesystem read-only, path no longer present, etc. The log entry could only say "Could not delete X", which is unactionable. Switch to Files.delete(Path), which throws an IOException carrying the underlying cause (NoSuchFileException, AccessDeniedException, or a generic IOException with the platform message). Log the exception along with the path so the operator has something to act on. Sonar S4042 flags exactly this pattern.
exportCharacterToNonPDF catches ExportException only to short-circuit to a false return; the comment "Error will already be reported to the log" makes the intent explicit. Naming the variable "e" suggested it might be used and forced a reader to scan the catch body to confirm it was not. Use the unnamed pattern (_) introduced in JEP 456 / Java 21 to declare the variable as deliberately unused. The compiler now enforces the intent and the discard is visible at the declaration site. Sonar S7467 flags exactly this case.
The original code converted the Path to a URI and then constructed a File from that URI. The intermediate URI step served no purpose: it introduced URI parsing and a second File constructor invocation while producing an equivalent File. Use Path.toFile() instead, which is the direct API for this conversion.
generateOutputFilename built the output name as
charname.substring(0, charname.lastIndexOf('.')) + '.' + extension
If charname has no dot, lastIndexOf returns -1 and substring(0, -1)
throws StringIndexOutOfBoundsException. Today this cannot happen in
practice because the only caller (exportCharacter / exportParty)
gates on PCGFile.isPCGenCharacterFile / isPCGenPartyFile, which
require .pcg or .pcp suffixes. But the guard is implicit and the
crash is a latent landmine if validation ever changes upstream.
Reuse removeFileExtension, which already handles the dotless case by
returning the input unchanged. Behaviour is identical for every name
that reaches this method today and safe for any name that might in
the future.
getValidFiles built its template filter as a chain of commons-io IOFileFilter wrappers: FileFilterUtils.asFileFilter(sheetFilter) + prefixFileFilter + and() + filterList. The SheetFilter enum already implements java.io.FilenameFilter, and the prefix check is a one-line String.startsWith — there is nothing here that needs the IOFileFilter abstraction. Combine the two checks into a single Predicate<File> lambda and run it through Stream.filter().sorted().collect(toList()) so the result matches the prior FileFilterUtils.filterList + Collections.sort semantics. This drops both commons-io.filefilter imports from the file and removes one more user of the dependency we are stripping.
SheetLoader located printable templates with FileUtils.listFiles plus a FileFilterUtils.and() chain of pdfFilter / suffixFilter / sheetFilter and a TrueFileFilter for directory descent. To do this it also implemented FilenameFilter purely so its accept() could be wrapped via FileFilterUtils.asFileFilter(this) — the class was serving as both worker and filter without need. Replace the listing with Files.walk(...) filtered by Files::isRegularFile and a single Predicate<File> that combines all three checks (parent dir name == "pdf", name does not end with .fo, name starts with the character template prefix). Drop the FilenameFilter implementation and the @NotNull import that came with it; the directory walk is recursive by default so no separate dir filter is needed. Failure mode is now explicit: an IOException from Files.walk is logged with the offending directory rather than swallowed inside commons-io. Removes the only remaining commons-io.* usage in this file.
With ExportUtilities, PrintPreviewDialog, BatchExporter and the rest of code/src/java migrated off org.apache.commons.io there are no remaining direct call sites — verified by grep across all source sets and by compiling main, test, itest and slowtest with the declarations removed. Drop the implementation declaration from build.gradle, the forceMerge entry under the jlink block, and the corresponding requires org.apache.commons.io from module-info.java. Commons-io may still be pulled transitively by FOP / batik on the runtime classpath, but our own module no longer reads it.
Rewrite the cleanup loop using java.nio.file: Files.list returns a Stream<Path>, so the temp-file scan, filter, and delete collapse into a try-with-resources block with two small helpers (isTemporaryExportFile, deleteQuietly). Behavioural improvements: - An unreadable temp directory is now logged, not silently treated as empty (the legacy listFiles returned null for both "not a directory" and "I/O error"). - A missing temp directory is handled explicitly via Files.isDirectory before listing, instead of relying on listFiles' implicit null. - Stream is closed deterministically by try-with-resources (matters on Windows file locking). This was the last in-tree usage of commons-io, so drop the dependency from build.gradle and the requires directive from module-info.java.
…me modes Replace xsl:use-attribute-sets="equipment.title" with an equivalent xsl:call-template name="attrib" call in fantasy_common.xsl (fantasy, 5e, starfinder, pathfinder_2, 4e, sagaborn) and killshot_common.xsl. Saxon-HE rejects references to undeclared attribute sets at compile time (XTSE0710); the attrib template is the correct runtime skinning mechanism. Add XsltPdfTemplateCompilationTest to guard against future regressions: compiles each fixed file directly under Saxon and fails on any XTSE0710/XTSE0720 error, ignoring expected fragment-level errors.
* Create separate action
* get the tag you created earlier
* upload zip artifacts for all platforms
* upload zip artifacts for all platforms
* Fix URIFactoryTest failing on Windows
Use File.toURI() instead of manually constructing a URI string with
new URI("file:" + path). On Windows, System.getProperty("user.dir")
returns backslash-separated paths (e.g. D:\a\pcgen\pcgen) which are
illegal characters in a URI, causing URISyntaxException.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Remove
commons-ioas a direct dependency by migrating its remaining call sites to JDK equivalents.Changes
BatchExporter— replacedTeeOutputStreamfan-out withByteArrayOutputStream+ sequentialFiles.write; cleaned upremoveTemporaryFiles, error reporting, and resource handling along the way.ExportUtilities.getValidFiles—IOFileFilter/FileFilterUtilschain → singlePredicate<File>+Stream.filter().sorted().PrintPreviewDialog.SheetLoader—FileUtils.listFiles+ filter chain →Files.walk+Predicate<File>; dropped the spuriousFilenameFilterself-implementation.build.gradle/module-info.java— dropped thecommons-ioimplementation declaration, thejlinkforceMergeentry, and therequires org.apache.commons.io.Test plan
./gradlew compileJava compileTestJava compileItestJava compileSlowtestJavapasses