search

flathub / com.synology.SynologyDrive

https://flathub.org/apps/details/com.synology.SynologyDrive
7 stars 10 forks source link

Update to 3.1.0-12923 #24

Closed RayJW closed 1 year ago

RayJW commented 2 years ago

Updates the deb package to the new 3.1.0-12923 version, released on 2022-04-06.

flathubbot commented 2 years ago

Started test build 111789

flathubbot commented 2 years ago

Build 111789 failed

RayJW commented 2 years ago

Closes #22

jibsaramnim commented 2 years ago

@RayJW it seems like the screenshot originally used has been re-saved by the Synology folk into a webp variant here. The original has been removed, which is what's causing the build to fail.

I was going to make a PR myself but I realized you had already done this. If you have a moment, perhaps you can change this in line 17 of com.synology.SynologyDrive.metainfo.xml?

flathubbot commented 2 years ago

Started test build 112655

RayJW commented 2 years ago

@jibsaramnim I updated my PR with a new commit containing the fix for the new image link. Lmk if there is anything else to do before merging :)

flathubbot commented 2 years ago

Build 112655 failed

jibsaramnim commented 2 years ago

I updated my PR with a new commit containing the fix for the new image link.

Welp, it still failed. Perhaps Flathub doesn't like webp images? I'm not sure.. we might have to wait for/ping someone in the know -- I'm only a (tiny) contributor myself. Thank you kindly for giving this a try though!

jibsaramnim commented 2 years ago

Ah yes, webp does not seem to be supported. I found this in the build logs of the failed build:

Failed to download https://www.synology.com/img/dsm/drive/seamlessly_01.webp: Couldn’t recognize the image file format for file “/srv/buildbot/worker/build-x86_64-7/build/.flatpak-builder/screenshots-cache/com.synology.SynologyDrive-seamlessly_01.webp”

I.. am not sure if there are built-in methods of converting something on the fly, or if we have to try to find an alternative host/source of a jpeg image instead. Hopefully someone in the know will be able to chime in on how to deal with this.

This is also what's preventing #25, so finding a fix for those should hopefully fix both PRs, which would be very nice indeed :).

RayJW commented 2 years ago

@jibsaramnim Thanks for investigating this. It's really weird because I couldn't find any other documentation or errors regarding webp images. Considering this codec is gaining quite some traction, I find it odd that no one else seems to have walked into this issue yet, but who knows. Closing both PRs would indeed be nice, as especially #25 is important because it might be an alarming error to users, and it's also just bad practice.

jibsaramnim commented 2 years ago

@hfiguiere Hi :wave:! you've helped me earlier this year with a version bump back then. I hope it's alright I'm tagging you directly; do you happen to know any way we might be able to get the screenshot download bit figured out so these two PRs can be approved and merged? It'd be especially great to get rid of the (admittedly rather ominous sounding) warning shown to all users regarding org.freedesktop.Sdk.

hfiguiere commented 2 years ago

webp isn't supported.

jibsaramnim commented 2 years ago

webp isn't supported.

Is it possible to include a jpeg screenshot directly in this repository and use that?

cstamas commented 2 years ago

Seems jpg exists also https://www.synology.com/img/dsm/drive/seamlessly_01.jpg

flathubbot commented 2 years ago

Started test build 113599

flathubbot commented 2 years ago

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

flatpak install --user https://dl.flathub.org/build-repo/111203/com.synology.SynologyDrive.flatpakref
RayJW commented 2 years ago

Seems jpg exists also synology.com/img/dsm/drive/seamlessly_01.jpg

Amazing, this seems to finally do the trick. Thank you for finding this @cstamas. I think we should now go ahead with merging this and #25 if possible to get this Flatpak up-to-date again @jibsaramnim.

RayJW commented 2 years ago

Tested and confirmed working on my system running Fedora 36 (GNOME). Thanks so much for making these changes!

image

Note that you'll have to pull in the latest from master once this PR is merged or apply the same fix in #25 for that one to be able to successfully build, too.

(I'm only a small contributor myself, so I can't merge this for you. But I hope my $0.02 are helpful here :)).

Perfect, thank you for testing! It also seems to work without issues on my system so far.

I will rebase #25 with main once this one is merged because I wanted to make them separate PRs to avoid any confusion or blockers with one or the other :)

jibsaramnim commented 1 year ago

Hi @hfiguiere! OP has fixed the webp reference and everything seems to be building and working great now. It would be really nice if this one could be merged so that #25 can be merged as-well (it's held up by the same image issue that's already fixed in this PR). Would you be able to help get these merged for us?

hfiguiere commented 1 year ago

build failed. :-(