Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better JVM detection #494

Merged
merged 11 commits into from
Jan 24, 2023
Merged

Better JVM detection #494

merged 11 commits into from
Jan 24, 2023

Conversation

ImUrX
Copy link
Member

@ImUrX ImUrX commented Jan 20, 2023

Added conditionals depending on OS, now Linux does it's own JVM detection based on /usr/lib/jvm.
Also left a macOS one because it makes the #[cfg()] a lot cleaner and I just need to actually test it (I know where macOS saves them but I still want to test it).
I prioritized the APPDIR env just in case, because if we are using AppImage, we should trust the jar we have instead of a relative dir one.
I also kind of crept in some optimizations on the release build for less bin size... 馃憖

@ImUrX ImUrX added Area: GUI Related to the GUI Type: Enhancement Adds or improves a feature OS: Linux Operating system: Linux labels Jan 20, 2023
@ImUrX ImUrX requested a review from TheButlah January 20, 2023 05:46
@ImUrX
Copy link
Member Author

ImUrX commented Jan 20, 2023

nvm, gonna delete the profile thing. It's getting ignored 馃槶

@TheButlah TheButlah marked this pull request as draft January 20, 2023 18:43
@ImUrX ImUrX marked this pull request as ready for review January 21, 2023 14:45
Copy link
Contributor

@TheButlah TheButlah left a comment

Choose a reason for hiding this comment

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

haven't tested it, but seems fine. Double check that the comments on line 301 are intended to be commented out @ImUrX

Copy link
Member

@kitlith kitlith left a comment

Choose a reason for hiding this comment

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

LGTM, though you might want to consider inspecting/respecting the JAVA_HOME environment variable at some point.

gui/src-tauri/Cargo.toml Outdated Show resolved Hide resolved
gui/src-tauri/src/main.rs Outdated Show resolved Hide resolved
gui/src-tauri/src/main.rs Outdated Show resolved Hide resolved
gui/src-tauri/src/main.rs Outdated Show resolved Hide resolved
gui/src-tauri/src/main.rs Show resolved Hide resolved
@ImUrX
Copy link
Member Author

ImUrX commented Jan 24, 2023

this is ready for merge

@Eirenliel Eirenliel merged commit f030496 into SlimeVR:main Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: GUI Related to the GUI OS: Linux Operating system: Linux Type: Enhancement Adds or improves a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants