YunoHost-Apps / photoprism_ynh

AI-Powered Photos App for the Decentralized Web
GNU Affero General Public License v3.0
7 stars 6 forks source link

Make installation compatible with arm64 and armhf architectures #19

Closed tituspijean closed 2 years ago

tituspijean commented 2 years ago

Closes #18

PR Status

Automatic tests

Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)

Thovi98 commented 2 years ago

Could you please tell me where you found the armhf sha256? I see only the ones for amd64 and arm64 in the Docker Hub

And I don’t understand how the script will find Photoprism (and the right version) with only the sha256, but that’s maybe my lack of knowledge 😁

tituspijean commented 2 years ago

It's in another tag: https://hub.docker.com/layers/photoprism/photoprism/photoprism/220730-armv7/images/sha256-4e0b39a06e0f7ecd11aa8cbd84c739cba3417c99348f52e3433cdeaeeb5474cb?context=explore

However, I realize now that armhf != armv7 :(

Limezy commented 2 years ago

Hi @tituspijean, do we have an arm64 CI ? I don't have any arm64 architecture to test your PR on right now...

tituspijean commented 2 years ago

Right now, no. I had tried to make one on my RPi4. I'll try to spin it up again during today's YunoCamp session.

Limezy commented 2 years ago

Ok ! Hard for me to make a review of your PR, but I let you merge whenever you feel it's OK @Thovi98 do you have a Pi or something to test on your side ?

Thovi98 commented 2 years ago

@Thovi98 do you have a Pi or something to test on your side ?

I’ve not, sorry :(

centralscrutinizer21 commented 2 years ago

I gave this branch a try without success https://paste.yunohost.org/raw/orenohogix

Limezy commented 2 years ago

@centralscrutinizer21 : Many thanks for testing, that will be very useful. It looks like there is still a few things to improve, I'll try to have a look.

tituspijean commented 2 years ago

I think I wrote a bad case block in _common.sh.

tituspijean commented 2 years ago

@centralscrutinizer21 you may try again. :)

centralscrutinizer21 commented 2 years ago

@tituspijean worked like a charm!

navanchauhan commented 2 years ago

Why don't we simply compile the program from scratch rather than using the docker-extractor? PhotoPrism looks like a simple program written in Go https://github.com/photoprism/photoprism/blob/develop/scripts/build.sh

I am happy to contribute the relevant PR that builds on the device itself if the package maintainer wants to go in that direction. All we would need to do is include a custom helper that installs Go -> Builds a static binary -> Uninstalls Go

I did that for ListMonk a few days back (ListMonk Repo)

tituspijean commented 2 years ago

As far as I know we moved to Docker image extraction because the compilation was taking A LOT of resources.

Limezy commented 2 years ago

Why don't we simply compile the program from scratch rather than using the docker-extractor? PhotoPrism looks like a simple program written in Go https://github.com/photoprism/photoprism/blob/develop/scripts/build.sh

I am happy to contribute the relevant PR that builds on the device itself if the package maintainer wants to go in that direction. All we would need to do is include a custom helper that installs Go -> Builds a static binary -> Uninstalls Go

I did that for ListMonk a few days back (ListMonk Repo)

Thanks for your proposal ! This is actually how I did package Photoprism before. It's way harder than that in fact, because of the dozens and dozens of required dependancies, not all using the Go system. You may have a look at past versions of the master branch if you want.

As far as I know we moved to Docker image extraction because the compilation was taking A LOT of resources.

Yep exactly ! The CI was crashing every time. Feel free anyway to contribute to this package, every help is warmly welcomed. We are currently changing to have @Thovi98 as the official maintainer of the app

Limezy commented 2 years ago

!testme !bullseyetestme

yunohost-bot commented 2 years ago

:carousel_horse: Test Badge

yunohost-bot commented 2 years ago

[Bullseye :horse:] May the CI gods be with you! Test Badge

Limezy commented 2 years ago

These tests will not secure the fact that the package will work on arm64 or other architectures. However, if the tests are running on the amd64 CI, I'll consider this PR as non regressing, hence mergeable.