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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Use the zbus-backed of notify-rust #6332

Merged
merged 4 commits into from
Mar 31, 2023
Merged

Conversation

zeenix
Copy link
Contributor

@zeenix zeenix commented Feb 21, 2023

While zbus used to have too new MSRV, that is no longer the case for several months now and hence there is no good reason to avoid zbus dependency. The MSRV is tracked in the zbus CI even to avoid any accidental bumps. The MSRV of zbus is 1.60 currently, which is only 1 version above the MSRV of dbus-rs. Neither Rust 1.60 nor 1.59 is available in Ubuntu LTS or Debian unstable so this change doesn't introduce any distro issues AFAICT.

OTOH, using zbus eliminates dependency on a platform lib and hence enhances the portability and ease of building. Issues like these will be a thing of the past after this change:

#4373
#5007
#5466


Moreover, the latest release of notify-rust enables async APIs based on zbus' async support, in case that is useful for Tauri project in the future.

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

While zbus used to have too new MSRV, that is no longer the case for
several months now and hence there is no good reason to avoid zbus
dependency. The MSRV is tracked in the zbus CI even to avoid any
accidental bumps.

OTOH, using zbus eliminates dependency on a platform lib and hence
enhances the portability and ease of building. Issues like these will be a
thing of the past after this change:

tauri-apps#4373
tauri-apps#5007
tauri-apps#5466
@zeenix zeenix requested a review from a team as a code owner February 21, 2023 12:52
@FabianLars
Copy link
Member

FabianLars commented Feb 21, 2023

The MSRV of zbus is 1.60 currently

That unfortunately is too new. We are stuck with 1.59 on Tauri v1.X - i can't remember what we used to determine that version, maybe what some debian version used to build firefox(?) and even though they raised their version above 1.59 too we can't change our msrv policy in a minor release. or at least didn't want to the last time we talked about it.

That said, this change would be fine on the next branch where we will likely change the MSRV policy to something like "last X versions, updated on each minor release"

Edit: Depending on how long we want to keep releasing 1.X version we should probably discuss the msrv again, but it looks like we will focus more on 2.0 than minor releases 🤔

@zeenix
Copy link
Contributor Author

zeenix commented Feb 21, 2023

That unfortunately is too new. We are stuck with 1.59 on Tauri v1.X - i can't remember what we used to determine that version,

Well to be fair, that was also quite new at some point so I'm sure there was a very good reason. :)

maybe what some debian version used to build firefox(?) and even though they raised their version above 1.59 too we can't change our msrv policy in a minor release. or at least didn't want to the last time we talked about it.

IMO that seems a bit too strict. Not bumping MSRV in a patch release (e.g what time crate recently did) would be not only understandable, it would be a good policy. While strictly speaking MSRV bump is a (potentially) breaking change, as we know, not all breaking changes are major.

But it's your project so your rules. :)

That said, this change would be fine on the next branch where we will likely change the MSRV policy to something like "last X versions, updated on each minor release"

Edit: Depending on how long we want to keep releasing 1.X version we should probably discuss the msrv again, but it looks like we will focus more on 2.0 than minor releases thinking

I understand. It all depends on your project's priorities. I would only suggest humbly that you consider an MSRV bump. Perhaps bumping only by 1 version (1.60) for now then switch to the new MSRV policy after 2.0?

@amrbashir
Copy link
Member

amrbashir commented Feb 21, 2023

I think it should be fine to bump the MSRV in a minor release and 1.60 is almost 1 year old, that's old enough.

@zeenix
Copy link
Contributor Author

zeenix commented Feb 22, 2023

@amrbashir cool. Should I bump the MSRV as part of this PR?

@amrbashir
Copy link
Member

Fine by me but I'd rather wait for @lucasfernog to approve this first.

@lucasfernog
Copy link
Member

MSRV 1.60 would also allow us to bump another dependency (serde-with IIRC) so it would be nice. But I'd like to hear from @chippers.

@FabianLars
Copy link
Member

FabianLars commented Feb 22, 2023

i mean, i'm all for a msrv bump (my first message was not my opinion but a summary of our past 100 discussions about this), but we should at least clearly communicate this, preferably (long) before actually releasing a new tauri version with it and not just via a small mention in the release notes on release day. Just because we were super strict about this so far in 1.X 🤷

We could then also use this chance for a teaser about the MSRV policy changes in v2 to ease users into that.

@zeenix
Copy link
Contributor Author

zeenix commented Feb 22, 2023

but we should at least clearly communicate this, preferably (long) before actually releasing a new tauri version with it and not just via a small mention in the release notes on release day. Just because we were super strict about this so far in 1.X shrug

Fair enough. Anything I can help with? Should I bump the MSRV in the PR and then you folks handle the rest?

@chippers
Copy link
Member

chippers commented Feb 23, 2023

One of the reasons why 1.59 is the magic number we settled on was that Debian packages 1.59in their stable release for Firefox, and they have a policy that packages must be built by the other packages in that release. Our goal was to hopefully make it easier for Tauri applications to make it into Debian in the future, and this is just the current stable version. Unfortunately, Debian has entered their first soft freeze for the next release and that rustc version is only 1.63which means we will miss out on some good stuff like workspace dependency inheritance.

Usually when a package maintainer commits to packaging an application, it would be up to them to freeze dependencies and create patches to satisfy the MSRV requirements, but we wanted it to be dead easy for a Tauri application to be able to be accepted.

It's not a permanent solution, just our general goal to allow easier packaging and more likelihood a Tauri app would be packaged.

If we can improve our Linux distribution stuff then this matters a lot less. Once we can have e.g. flatpaks working very well and perhaps an easier way for generating 3rd party repos (like how installing some packages will actually add their own apt repository and pull the packages from there) I think we would be in a much better spot to adopt a stable - Xpolicy.

Since no new packages can make it into current or next stable, perhaps we just adopt testing's version for now (and upgrade as it upgrades) until we decide what to do later.

@chippers
Copy link
Member

@lucasfernog ^

@zeenix
Copy link
Contributor Author

zeenix commented Feb 28, 2023

Ping? ☺️

@chippers
Copy link
Member

chippers commented Mar 1, 2023

Ping? ☺️

Lucas is taking a break right now, so a decision will likely take a bit of time

@lucasfernog
Copy link
Member

I feel like we should discuss the MSRV bump with the rest of the community.

@zeenix
Copy link
Contributor Author

zeenix commented Mar 17, 2023

I feel like we should discuss the MSRV bump with the rest of the community.

Tbh, I was hoping you'd just be the last approver to bumping and we can finally get this landed, but sure. Please let me know how I can help with that.

@zeenix
Copy link
Contributor Author

zeenix commented Mar 31, 2023

I feel like we should discuss the MSRV bump with the rest of the community.

How is that going?

@lucasfernog lucasfernog requested a review from a team as a code owner March 31, 2023 13:44
@zeenix
Copy link
Contributor Author

zeenix commented Mar 31, 2023

thanks @lucasfernog

@lucasfernog lucasfernog merged commit 5fdc616 into tauri-apps:dev Mar 31, 2023
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.

5 participants