AppImage Qt5.8

[rullzer]

• Dockerfile to have an image with the basics (Qt5.8 etc)
• build.sh to build it all

So the steps:

1. docker build -t nextcloud-client-appimage:5.8 linux/AppImage
2. docker run -t -i --rm --cap-add SYS_ADMIN --cap-add MKNOD --device=/dev/fuse --security-opt apparmor:unconfined -v "$PWD:/home/client" nextcloud-client-appimage:5.8
3. /home/client/linux/AppImage/build.sh

@probonopd as discussed.
Maybe we should merge this with the already running stuff on travis? (Or just use this to build and add a container to our drone ci?)

Feedback welcome.

[probonopd]

So the Travis CI AppImage build still uses Qt 4 while the Docker stuff uses 5.8.

Did you know that you can run Docker images on Travis CI? I think that'd be the way to go: Have Travis CI use the Docker image to build the AppImage. Do you think you can do this? It would also save me from setting up a Docker instance locally on my machine.

[TheAssassin]

Bad idea to do it like this @rullzer. You should NOT enable FUSE in Docker with black magic, bypassing several security measurements enforced by default by Docker for good reasons.

Instead, --appimage-extract the AppImage(s) you use inside, and call squashfs-root/AppRun *any arg you need*.

Also, you can just integrate it with your Travis file, as you can run Docker stuff on Travis IIRC.

[TheAssassin]

Oh, btw, you should also ADD your build script, and call it as CMD. Then you just have to docker run --rm it, which can be done headlessly.

If you need more information on how to do it right™, please get in touch with me on Freenode in #appimage.

[enoch85]

cc @ivaradi

[ivaradi]

This branch has been made from a few commits behind the current master, i.e. it does not contain my fix for the determination of the base version (i.e. 2.3.1 or 2.3.2). This is why the "push" build fails, but the "pr" one is successful. So I think the build failure can be ignored in this case.

[rullzer]

Thnx @TheAssassin for the pointers. I'll get to work with those ;)

[rullzer]

@ivaradi @enoch85 yeah my bad. Rebased and all good now

[rullzer]

@TheAssassin I fixed it a bit. Now no more priviledges to docker are required thnx for the tip.

Now lets see if I can make @probonopd stuff work with Qt5.8!

[enoch85]

Hmm, what happened to the automatic Snap builds? cc @ivaradi @3v1n0

[3v1n0]

@enoch85 what you mean? I see the snap to be sitll built even in this PR.

If you mean the upload to the store, I'm still waiting for the small work in your side that @oparoz should do (or anyone else who can access to the nextcloud account in the snap store).

[enoch85]

@3v1n0 This is what I see:

It used to be "Pre-release" before.

Yeah, would be great if someone could fix the store issue.

[3v1n0]

@enoch85 mh... I see, i guess (I've not checked the logs yet) it's because of this security advisor.

[enoch85]

Any news here?

[TheAssassin]

One last issue that should probably be fixed:

In the build script, the option -D CMAKE_INSTALL_PREFIX=/app is used. We recommend using /usr instead, and then use make install DESTDIR=/app. See the linuxdeployqt README for more information.

Other than that, it looks good to me now.

[enoch85]

Any news here?

@TheAssassin Could you please add the suggested changes?

[probonopd]

@rullzer is this still what you are using? Looks like this is not using linuxdeployqt at all, is this intentional?

[TheAssassin]

@probonopd no it does not use linuxdeployqt, unfortunately.

@enoch85 I cannot push to the repository so I cannot update that specific branch, I am just a collaborator because I sent a PR previously. Talking about PRs, please see #181.

[enoch85]

@TheAssassin @probonopd So this is OK to merge now from your perspective?

[TheAssassin]

Well, @probonopd is right that the recipe should use linuxdeployqt instead of the legacy shell script stuff. So, technically yes, you can merge it as is after checking whether the latest build works, and change it to use linuxdeployqt eventually.

[enoch85]

@TheAssassin Sure, let's make another PR for the fixes.

Thanks for confirming!

[TheAssassin]

Have you actually run it...? Just wondering...

[enoch85]

@TheAssassin Nope, trusted you that it worked. All the checks passed and you approved it so I figured everything was OK?

[TheAssassin]

I just thought a few further checks wouldn't hurt.

[probonopd]

It's broken... starting here https://travis-ci.org/nextcloud/client_theming/jobs/251305311#L2107... should be /app/usr/share/... rather than /app/share/...

[enoch85]

@TheAssassin You're right, I was a bit quick. 😟

@probonopd Could you please prepare another PR?

[probonopd]

Working on it in #182

[TheAssassin]

I see... I ran the old version instead of the one with the changes, and failed to notice the mistake... sorry for that.

Pro tip: always check the results of a PR yourself before merging. And don't submit code that isn't tested thoroughly.

Thanks @probonopd, let me know if I can support your efforts. You might visit IRC once in a while by the way, there's news.