Skip to content

refactor: ⬆️ Update dependencies#3243

Merged
zmerp merged 1 commit into
masterfrom
deps-update
Apr 14, 2026
Merged

refactor: ⬆️ Update dependencies#3243
zmerp merged 1 commit into
masterfrom
deps-update

Conversation

@zmerp

@zmerp zmerp commented Mar 29, 2026

Copy link
Copy Markdown
Member

Update all dependencies that don't require big breaking changes.
Exceptions to this are:

  • jni:
    • refactored using new idiomatic conventions
    • Correctness fix: replaced JObject::from_raw(context) with JObject::global_kind_from_raw(context)
  • egui

I've also changed the version number convention showing all major, minor and patch numbers.

@zmerp zmerp force-pushed the deps-update branch 7 times, most recently from 8f66add to 8931b09 Compare April 1, 2026 08:28
@zmerp

zmerp commented Apr 1, 2026

Copy link
Copy Markdown
Member Author

@duck I'm still sorting out some issues, as the client gets stuck at startup. Let me know if you have anything to say (bugs or just style). I think I'll merge when I resolved the client issue

@The-personified-devil

Copy link
Copy Markdown
Collaborator

What's the reasoning behind locking down to patch versions? I'm not sure if that actually fully freezes the dependency and otherwise we should just stay with major/0.minor and actually properly use the lockfile...

@The-personified-devil The-personified-devil left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, but I would like some reasoning on the change to nailing down patch versions, that seems like it'd just cause us to have to update deps more often...

@zmerp

zmerp commented Apr 8, 2026

Copy link
Copy Markdown
Member Author

@The-personified-devil the reason I did this is because most contributors pin to the patch version, because various tools do this like "cargo add" or Dependi on vscode, even though I tried to keep this consistent to major.patch. Online I haven't seen much consensus about what is more idiomatic, except that most projects do use major.minor.patch.

@zmerp

zmerp commented Apr 11, 2026

Copy link
Copy Markdown
Member Author

It seems that hitting cargo update even without any other changes it triggers the crash. Nothing on the logcat unfortunately. This is going to take a while

@zmerp

zmerp commented Apr 12, 2026

Copy link
Copy Markdown
Member Author

I haven't gone to the bottom of this, but I've made some progress. tried to selectively invoke cargo update for single dependencies, until I found that android-activity 0.6.1 triggered the freeze (up from 0.6.0). After bisecting the commits of this release I found that it was the change of the handling of ndk-context initialization, where before it was initialized with an Activity pointer, and now it is initialized with an Application Context pointer. This is more correct given the invariants of ndk-context, but some downstream dependency instead assumed the pointer to be an Activity, hence the crash. I found 3 possible dependency culprits: app_dirs2, openxrs, and out own ndk code. I commented out all of our ndk code and app_dirs2 code, they are not the culprit. The remaining one is openxrs, but in its code I see that clearly it needs an application context pointer and not an activity context pointer. So at this point the issue is either the Quest runtime requires an Activity pointer (breaking the OpenXR spec) or I've made some other errors. From the limited log analysis I did I found that logs seem to reach the innermost OpenXR loop, so the session is initialized, but no rendering is done (screen stays on blinking 3 dots) and no apparent crash is visible in the log.

So while I could continue debugging, for time sake I will just pin android-activity to 0.6.0 for now.

@zmerp

zmerp commented Apr 13, 2026

Copy link
Copy Markdown
Member Author

@The-personified-devil I've reverted the inclusion of patch version. I've seen that many big crates use major.minor, albeit sometimes including the patch of some dependencies for no apparent reason, probably because of external contributors not respecting the convention

@zmerp zmerp merged commit 750b73c into master Apr 14, 2026
12 checks passed
@zmerp zmerp deleted the deps-update branch April 14, 2026 21:24
@Timbals

Timbals commented Apr 22, 2026

Copy link
Copy Markdown

@zmerp The issue is very likely caused by openxrs assuming the context is an activity.
It will be fixed when Ralith/openxrs#217 is merged and a new version is released.

@zmerp

zmerp commented Apr 22, 2026

Copy link
Copy Markdown
Member Author

@Timbals thanks for your work!

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.

3 participants