Skip to content

Conversation

MGough
Copy link
Contributor

@MGough MGough commented Apr 11, 2025

Upgrades notify-rust to a patched version as the previous allowed versions included one which resulted in a panic on MacOS when any notification was sent. See #2431

Verified as working following these changes, it no longer just panics (which was crashing Tauri):
Screenshot 2025-04-11 at 16 06 51

My first contribution to this repo, so let me know if there's something I've done wrong or missed, or feel free to tweak this

@MGough MGough requested a review from a team as a code owner April 11, 2025 15:09
@MGough

This comment was marked as outdated.

@MGough MGough force-pushed the update-notification-notify-rust branch 4 times, most recently from ea316cb to 26deda9 Compare April 11, 2025 15:46
@MGough MGough changed the title fix(notifications): update notify-rust to ^4.11.7 to resolve panic on MacOS (fixes #2431) fix(notifications): update notify-rust to 4.11.7 to resolve panic on MacOS (fixes #2431) Apr 11, 2025
@MGough
Copy link
Contributor Author

MGough commented Apr 11, 2025

Seems based on the CI perhaps my version of rust is too recent? Advice welcome on how to manage that, I'm just running what we're using for our Tauri builds but perhaps I need to run something lower for generating the lockfile?

Updated: Just working through the CI issues now - will use a rust toolchain to run using 1.77.2

@FabianLars
Copy link
Member

The fix was in mac-notification-sys and notify-rust is not locking its version so the Cargo.toml change doesn't do anything really.
The Cargo.lock file in this repo does not affect plugin consumers so this is also irrelevant outside of the repo examples.

If you remove the Cargo.toml change (we typically don't lock patch versions to give users a bit more freedom) and the changefile I'd still merge it because I like the lock file changes for ci

@MGough
Copy link
Contributor Author

MGough commented Apr 14, 2025

The fix was in mac-notification-sys and notify-rust is not locking its version so the Cargo.toml change doesn't do anything really. The Cargo.lock file in this repo does not affect plugin consumers so this is also irrelevant outside of the repo examples.

If you remove the Cargo.toml change (we typically don't lock patch versions to give users a bit more freedom) and the changefile I'd still merge it because I like the lock file changes for ci

Long week got to me! I'd originally intended it as enforcing a minimum version for MacOS rather than locking to the patch version, I appreciate that's not what my PR does. It seems desirable to me from an end user's perspective to ensure that the plugin doesn't resolve to a version of notify-rust that we know is broken?

Would the tauri team agree on that, or do you prefer to leave it totally open and assume that the end-user will be retrieving the latest version of notify-rust?

@MGough MGough force-pushed the update-notification-notify-rust branch from 0a0884a to 45940f9 Compare April 14, 2025 09:08
@MGough MGough changed the title fix(notifications): update notify-rust to 4.11.7 to resolve panic on MacOS (fixes #2431) chore(notifications): regen lockfile bumping notify-rust to 4.11.7 to resolve panic on MacOS Apr 14, 2025
@FabianLars
Copy link
Member

In theory i'd be open to set a min version (for mac-notification-sys since that was broken) but i'd rather risk it if it means i don't get screamed at again for setting the min minor/patch versions too high 🙃

@MGough
Copy link
Contributor Author

MGough commented Apr 14, 2025

In theory i'd be open to set a min version (for mac-notification-sys since that was broken) but i'd rather risk it if it means i don't get screamed at again for setting the min minor/patch versions too high 🙃

Understandable - I've seen both sides! I'm not pushing for it, as I know ultimately it'll be the core tauri team that deals with the fallout. Feel free to either close out this PR or merge the lockfile change if you're happy. Thanks as always Fabian (& Lucas)!

@FabianLars
Copy link
Member

i'll merge this as-is then and keep monitoring reports about 2431 - thanks :)

Copy link
Contributor

Package Changes Through 45940f9

There are 2 changes which include log with minor, log-js with minor

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package current next
api-example 2.0.24 2.0.25
api-example-js 2.0.20 2.0.21
log 2.3.1 2.4.0
log-js 2.3.1 2.4.0

Add another change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

@FabianLars FabianLars merged commit f81e800 into tauri-apps:v2 Apr 14, 2025
144 checks passed
gezihuzi pushed a commit to Hypobenthos/plugins-workspace that referenced this pull request Jun 22, 2025
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.

2 participants