-
Notifications
You must be signed in to change notification settings - Fork 281
macOS ml support #2158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
macOS ml support #2158
Conversation
- Use Java 24 for photonvision app builds (build-gradle, playwright-tests, build-package, run-smoketest-native) - Keep Java 17 for photonlib builds (build-examples, build-photonlib-host) for FRC compatibility - Exclude SwiftJava samples from CI to prevent jextract failures - Add PhotonAppleLibraryLoader stub for non-macOS platforms to fix Linux compilation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- Remove SwiftKit publish from photonlib jobs (build-examples, build-photonlib-host) - Add photonAppleForceStubs property to use stubs even on macOS for photonlib builds - Disable photon-apple test compilation on non-macOS (tests require Java 24 APIs) - Only photon-server builds on macOS use real photon-apple implementation This ensures photonlib (which targets Java 17 for FRC) doesn't depend on Java 24 or Swift libraries, while photon-server can still use native Apple object detection on macOS. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
JaCoCo 0.8.10 doesn't support Java 24 class files (major version 68). Upgrading to 0.8.13 which supports Java 24. Fixes jacocoTestReport task failure: "Unsupported class file major version 68" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- SwiftKit requires Java 25 minimum - Java 25 is the LTS release - Update photon-apple to use Java 25 for real implementation and stubs - photon-apple uses Java 17 when photonAppleForceStubs is set (for photonlib) - Update CI workflows to install Java 25 instead of Java 24 - Update CLAUDE.md documentation Fixes build failures: - macOS: "swiftkit-ffm:1.0-SNAPSHOT is only compatible with JVM runtime version 25 or newer" - Windows/Linux: Java 25 installed but photon-apple was looking for Java 17 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- Java 24 works locally, no need for Java 25 - Java 25 requires Gradle 9 upgrade (not ready for that) - SwiftKit submodule hardcodes Java 25 in build.gradle - Add Gradle init script in CI to force SwiftKit to use Java 24 toolchain - FFM API is in preview in Java 24 (already using --enable-preview) - Revert photon-apple from Java 25 back to Java 24 - Revert CI workflows from Java 25 back to Java 24 - Update CLAUDE.md documentation This avoids the Gradle major version upgrade while keeping builds working. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
SwiftKit requires Java 25 and is only needed on macOS builds. Changes: - build-package job: Install Java 25 on macOS, Java 24 on Linux/Windows - photon-apple: Use Java 25 on macOS (real impl), Java 24 on Linux/Windows (stubs) - PhotonLib: Still uses Java 17 with photonAppleForceStubs flag - Remove Gradle init script workaround (no longer needed) This fixes the macOS build failure where SwiftKit libraries compiled with Java 25 couldn't be used by Java 24 builds. Gradle 8 supports both Java 24 and Java 25, no Gradle upgrade needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
The issue: Gradle 8.14.3 can't RUN on Java 25 (only supports up to Java 24). However, Gradle 8 CAN use Java 25 as a toolchain for compilation. Error was: "Unsupported class file major version 69" (Java 25) Solution: - Install Java 24 FIRST (sets JAVA_HOME to Java 24) - Install Java 25 SECOND on macOS (available as toolchain) - Gradle runs on Java 24 (via JAVA_HOME) - photon-apple compiles with Java 25 toolchain (via gradle toolchain API) This matches local development where Gradle runs on Java 24 but can still compile code targeting Java 25. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
The actions/setup-java action always sets the newly installed version as the default (JAVA_HOME). By installing Java 25 first, then Java 24 second, we ensure JAVA_HOME points to Java 24 (which Gradle 8 can run on) while keeping Java 25 available as a toolchain for SwiftKit compilation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Set sourceCompatibility/targetCompatibility to Java 24 for photon-apple to make it compatible with Java 24 consumers during Gradle dependency resolution. Compilation still uses Java 25 toolchain for SwiftKit. This fixes the error: "Dependency resolution is looking for a library compatible with JVM runtime version 24, but 'project :photon-apple' is only compatible with JVM runtime version 25 or newer." 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Change from excluding specific sample to excluding all :Samples:build. The swift-java submodule uses Gradle 9 which has stricter parsing, causing build failures in sample projects we don't need to build. Fixes SwiftJavaExtractFFMSampleApp JSON parsing error in CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Document the sourceCompatibility/targetCompatibility approach for photon-apple to resolve the dependency resolution vs runtime requirements conflict. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Update swift-java submodule to commit fe857f6156c319c9c424e457e9529973d0de308d which includes Swift libraries loading from JAR resources. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
| def forceStubs = project.hasProperty('photonAppleForceStubs') | ||
| def useRealImplementation = isMacOS && !forceStubs | ||
|
|
||
| // Configure Java version based on build mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we document somewhere the reason driving all this trickery around java versions? I know some of this is in Discord, but would like this captured
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documented in docs/source/docs/contributing/building-photon.md
| /** Cleaner instance to release the detector when it goes out of scope */ | ||
| private final Cleaner cleaner = Cleaner.create(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other classes use Releaseable because we don't have control over when this cleanup happens. Prefer to manually destroy objects rather than relying on GC to do this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of it is cleaned up manually after each detection. The detector object itself is long lived and GC'd. There were thread safety issues otherwise.
| // Completely unsupported | ||
| MACOS("Mac OS", Platform::getUnknownModel, false, OSType.MACOS, true), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if somewhat supported, correct this comment
CLAUDE.md
Outdated
| On macOS, you must manually publish the SwiftKit libraries to your local Maven repository before building: | ||
|
|
||
| ```bash | ||
| cd photon-apple/swift-java | ||
| ./gradlew publishToMavenLocal | ||
| cd ../.. | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels goofy. the whole point of submodules and gradle is that they can be resolved and built as part of the overall build right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So swift-java isn't available on Maven :(. They haven't setup publishing. And they only support building with Gradle 9 so it has to be done with its own gradle daemon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we not patch their build for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How much extra work is this for both Mac and non Mac users trying to build photon? Will they need to go read docs and remember to publish this magic library to maven local?
I feel like a better solution is to include the library as part of our build system, and have photon-apple depend on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only needs to be published on mac.
And even on mac, if you use the flag -PphotonAppleForceStubs on any of the Gradle commands, it will build on Mac with all of swift turned off just like on windows. Everything should work except any object detection vision threads will crash.
The macOS CI build was taking 40 minutes with 30 minutes spent building swift-java. This adds caching for the photon-apple/swift-java/.build directory to significantly speed up subsequent builds. The cache key is based on Package.swift, gradle.properties, and settings.gradle to ensure the cache is invalidated when dependencies change.
- Update macOS build matrix to use aarch64/macarm64 for primary macOS job - Change cache keys from runner.os to matrix.artifact-name for proper architecture separation - Remove debug output now that Swift build directory locations are confirmed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Instead of managing jni in a separate repo, this takes the approach of monorepoing even platform specific native libs.
photon-apple builds swift code specific to macos.
On macos, it actually builds the swift code.
On non-macos, it uses stubs with matching types and declarations.
This way, photon-core can use apis in photon-apple without reflection and the classes will resolve.
This approach can be used to monorepo more native things in the future.
note: Swift does support linux arm64 so we could use it for linux too. Swift has c++ interop so it could be used as a nicer bridging layer where Java -> Swift is generated by swift-java over FFM/JNI and Swift->C++ is handled by the swift runtime so y'all don't have to touch jni.