flathub / us.zoom.Zoom

https://flathub.org/apps/details/us.zoom.Zoom
35 stars 44 forks source link

Try Zoom 5.17.1 with latest Zypak #439

Closed takluyver closed 6 months ago

takluyver commented 6 months ago

And set ZYPAK_CEF_LIBRARY_PATH environment variable, added in https://github.com/refi64/zypak/commit/c7daf0cc5b82eb5f63a1665cf7d72ff267100690

I don't know if this will fix the crashes; I'll test it once built.

flathubbot commented 6 months ago

Started test build 93728

flathubbot commented 6 months ago

Build 93728 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/76475/us.zoom.Zoom.flatpakref
takluyver commented 6 months ago

That does seem to solve the crashes.

Building Zypak here is redundant with the one in the Electron Baseapp. I've opened a PR to update it there, but the maintainers understandably want a tagged version, which so far hasn't happened. :shrug:

flathubbot commented 6 months ago

Started test build 93731

flathubbot commented 6 months ago

Build 93731 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/76478/us.zoom.Zoom.flatpakref
pgoodall commented 6 months ago

@takluyver That build works for me, too. Thanks for your work on this. I guess we need to wait for your fix to be accepted upstream in Zypak?

takluyver commented 6 months ago

The fix is already in Zypak (nothing to do with me!), but not in a tagged release. We can easily build it without a tag, it's just neater to have a tag.

On Wed, 17 Jan 2024, 14:34 Pete Goodall, @.***> wrote:

@takluyver https://github.com/takluyver That build works for me, too. Thanks for your work on this. I guess we need to wait for your fix to be accepted upstream in Zypak?

— Reply to this email directly, view it on GitHub https://github.com/flathub/us.zoom.Zoom/pull/439#issuecomment-1895938913, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACQB5J7XGN3LLQXPOQ2QBTYO7OQJAVCNFSM6AAAAABB4WOH6KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJVHEZTQOJRGM . You are receiving this because you were mentioned.Message ID: <flathub/us. @.***>

nedrichards commented 6 months ago

This lgtm - unless @muelli disagrees I'll merge this in a day or so. We can pull the build of zypak out when a release is made upstream but we can't ship and older version of zoom forever.

muelli commented 6 months ago

unless @muelli disagrees I'll merge this in a day or so.

I don't object at all. My only suggestion is to put a comment with a reference to a bug or something next to the added line, in case the line is only required or as long as the older zypak is used, so that we will know what use that line has.

takluyver commented 6 months ago

The new version has been tagged and included in the Electron baseapp now. I believe we still need to set the extra environment variable, so I've cherry-picked that commit onto a new branch (#440).