Skip to content

fix(parser): eliminate Kotlin coroutine bytecode causing ClassNotFoundException in Gradle plugin#281

Merged
rafaeltonholo merged 10 commits intomainfrom
fix/gradle-classloader-spilling-kt
Apr 1, 2026
Merged

fix(parser): eliminate Kotlin coroutine bytecode causing ClassNotFoundException in Gradle plugin#281
rafaeltonholo merged 10 commits intomainfrom
fix/gradle-classloader-spilling-kt

Conversation

@rafaeltonholo
Copy link
Copy Markdown
Owner

@rafaeltonholo rafaeltonholo commented Apr 1, 2026

Summary

  • Replace all sequence {} builders in commonMain with explicit stack-based iterators to prevent ClassNotFoundException for kotlin.coroutines.jvm.internal.SpillingKt when running inside Gradle 8.x, which bundles an older Kotlin stdlib (pre-2.1.20)
  • Add depthFirstSequence utility for coroutine-free tree traversal, used by listRecursively and deleteRecursivelyCompat
  • Refactor deleteRecursivelyCompat from .toList().asReversed() to stack-based post-order traversal, reducing memory usage from O(n) to O(depth) for large directory trees
  • Replace sequence {} in applyTransformation with an explicit Iterator to eliminate the last SpillingKt reference
  • Remove unused resolveSymlinks/symlinkTarget helpers and followSymlinks parameter

Summary by CodeRabbit

  • Refactor
    • Optimized file listing and deletion to use iterative depth-first traversal, with explicit depth limits and no symlink following.
    • Improved behavior when deleting non-existent targets (optional error vs. no-op).
  • Tests
    • Added comprehensive tests for listing and recursive deletion, including depth limits, ordering guarantees, laziness, and error cases.

…azy iterator

Okio's sequence builder and our custom listRecursively both compile to coroutine bytecode referencing SpillingKt (added in Kotlin 2.1.20).
Gradle 8.x bundles an older Kotlin stdlib, causing ClassNotFoundException at runtime inside the Gradle plugin.

- Replace with lazySequence utility backed by an explicit Iterator stack.
- Also add deleteRecursivelyCompat for the same reason
Wire the coroutine-free deleteRecursivelyCompat into FileManager so the Gradle plugin no longer calls Okio's deleteRecursively (which uses sequence{} internally).
…t helpers entirely.

Also adds an inline comment explaining the depth computation formula and fixes a truncated KDoc sentence in deleteRecursivelyCompat.
…sivelyCompat

Add tests for non-existent directory, deep tree traversal, and empty
directory deletion to improve coverage of filesystem extension functions.
…er traversal

Replace .toList().asReversed() with in-place stack-based deletion so
memory usage scales with tree depth, not total node count. Files are
deleted immediately on visit; directories are deleted once empty.
Document that DepthFirstIterator is not thread-safe and that sharing
a single iterator across threads will corrupt the internal stack.
…Iterator

Eliminates the last sequence{} builder in commonMain, which caused
ClassNotFoundException for SpillingKt when running inside Gradle 8.x.
The old name was too generic and suggested a general-purpose lazy
sequence builder. The new name communicates that it performs a
depth-first tree traversal specifically.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7093f920-1569-45f2-bab2-255e5495cbfd

📥 Commits

Reviewing files that changed from the base of the PR and between 8d2e99e and 86d78f9.

📒 Files selected for processing (1)
  • svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/extensions/FileSystemDeleteRecursivelyCompatTest.kt
✅ Files skipped from review due to trivial changes (1)
  • svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/extensions/FileSystemDeleteRecursivelyCompatTest.kt

Walkthrough

Introduces a stack-based depth-first traversal utility, rewrites sequence builders to explicit iterators, simplifies listRecursively (removing symlink-following), adds deleteRecursivelyCompat (iterative post-order deletion with existence handling), and adds tests covering traversal and deletion behaviors.

Changes

Cohort / File(s) Summary
Build Dependencies
svg-to-compose/build.gradle.kts
Added libs.com.squareup.okio.fakefilesystem to commonTest for filesystem tests.
Sequence Traversal Utilities
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/LazySequence.kt
Added depthFirstSequence(seeds, childrenOf) with a stack-backed DepthFirstIterator implementing lazy depth-first pre-order traversal (no coroutine sequence builder).
FileSystem Extensions
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/FileSystem.extension.kt
Reworked listRecursively(dir, maxDepth?) to use depthFirstSequence, removed followSymlinks param and symlink-handling; added deleteRecursivelyCompat(fileOrDirectory, mustExist=false) implementing iterative post-order deletion and explicit non-existence handling.
Transform Logic Refactoring
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/geom/ApplyTransforms.kt
Replaced coroutine sequence {} implementation with an explicit Sequence { Iterator<...> } iterator for List<PathNodes>.applyTransformation, preserving transformation semantics.
FileManager Integration
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/io/FileManager.kt
deleteRecursively now delegates to fileSystem.deleteRecursivelyCompat(...) instead of the previous deleteRecursively call.
Traversal and Deletion Tests
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/extensions/FileSystemListRecursivelyTest.kt, .../FileSystemDeleteRecursivelyCompatTest.kt
Added tests validating listRecursively behavior (maxDepth, empty/non-existent dirs, deep trees) and deleteRecursivelyCompat semantics (child-before-parent deletion ordering, streaming vs materialization, mustExist handling).
Sequence Utility Tests
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/extensions/LazySequenceTest.kt
Added tests for depthFirstSequence covering empty seeds, flat seeds, tree preorder traversal, and laziness (children callback invoked only as needed).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: eliminating Kotlin coroutine bytecode to fix ClassNotFoundException in Gradle plugin, which aligns with the PR's core objective of replacing sequence { } builders with stack-based iterators.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/gradle-classloader-spilling-kt

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/extensions/FileSystemDeleteRecursivelyCompatTest.kt (1)

34-34: Consider using assertFalse for negated existence checks.

assertTrue(!fs.exists(path)) works but assertFalse(fs.exists(path)) is more idiomatic and produces clearer failure messages.

♻️ Example change
-        assertTrue(!fs.exists(root))
+        assertFalse(fs.exists(root))

Also applies to: 63-63, 74-74, 119-119

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/extensions/FileSystemDeleteRecursivelyCompatTest.kt`
at line 34, Replace negated assertTrue checks with assertFalse for clearer,
idiomatic tests: change assertions like assertTrue(!fs.exists(root)) to
assertFalse(fs.exists(root)), and do the same for the other occurrences using
fs.exists (the instances referenced around the same test file: the assertions at
the other locations that call assertTrue with a negated fs.exists). Ensure you
only swap the assertion call (assertTrue(!...) -> assertFalse(...)) and keep the
fs.exists(...) expression unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/extensions/FileSystemDeleteRecursivelyCompatTest.kt`:
- Around line 122-126: The assertEquals call in
FileSystemDeleteRecursivelyCompatTest has its expected and actual arguments
reversed; change the parameters so expected = expectedDeletions and actual =
recorder.deletedPaths.size (i.e., swap them in the assertEquals invocation) to
ensure failure messages report the expected value correctly and reference the
correct symbols (recorder.deletedPaths.size and expectedDeletions) when updating
the test.

---

Nitpick comments:
In
`@svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/extensions/FileSystemDeleteRecursivelyCompatTest.kt`:
- Line 34: Replace negated assertTrue checks with assertFalse for clearer,
idiomatic tests: change assertions like assertTrue(!fs.exists(root)) to
assertFalse(fs.exists(root)), and do the same for the other occurrences using
fs.exists (the instances referenced around the same test file: the assertions at
the other locations that call assertTrue with a negated fs.exists). Ensure you
only swap the assertion call (assertTrue(!...) -> assertFalse(...)) and keep the
fs.exists(...) expression unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 466cb1cf-b6a9-40a5-919c-d8509d2d4281

📥 Commits

Reviewing files that changed from the base of the PR and between 1167695 and 8d2e99e.

📒 Files selected for processing (8)
  • svg-to-compose/build.gradle.kts
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/FileSystem.extension.kt
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/LazySequence.kt
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/geom/ApplyTransforms.kt
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/io/FileManager.kt
  • svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/extensions/FileSystemDeleteRecursivelyCompatTest.kt
  • svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/extensions/FileSystemListRecursivelyTest.kt
  • svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/extensions/LazySequenceTest.kt

@rafaeltonholo rafaeltonholo merged commit ed06233 into main Apr 1, 2026
4 checks passed
@rafaeltonholo rafaeltonholo deleted the fix/gradle-classloader-spilling-kt branch April 1, 2026 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant