-
Notifications
You must be signed in to change notification settings - Fork 266
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
Initial set of changes required for publishing to Flathub #293
base: develop
Are you sure you want to change the base?
Conversation
The `curl -L https://yarnpkg.com/latest.tar.gz` command fails on SSL validation.
cc @dbkr My current understanding of element-builder is that Linux builds are executed within the element-desktop-dockerbuild Docker image, so this PR adds dependencies required to run flatpak and flat-manager-client (with the latter being used to do the actual upload to Flathub). I'm still working through https://github.com/vector-im/element-builder/ to understand what needs changing there. |
Although I'm having some second thoughts, looking at https://github.com/vector-im/element-builder/blob/main/src/desktop_develop.ts#L289. I will continue with the assumption that publishing to Flathub part will happen from build environment to avoid polluting the host with "random" dependencies for now. |
Ubuntu 16.04 doesn't ship Flatpak in default repositories, so we need to install it from the project's PPA. Additionally, it ships too old Python for a tool used to create and upload Flathub builds to work, so we need another PPA to install Python 3.6.
It is easier to create a flatpak package from "raw" files than having to additionally extract a deb file.
I've also started looking into the backstory of using Ubuntu 16.04 in Dockerfile. #84 seems to imply there were some issues caused by discrepancy between glibc version inside the container and on Debian 9, but it doesn't seem to be the case? electron-builder doesn't build Electron from scratch and downloads a pre-built binary instead, and it seems to require glibc 2.17 to work (which is much older than what Xenial ships). This is executed on my Fedora 33:
Mentioning this as using so old container for effective glue operations seems to make things more difficult than it's worth it; also 16.04 is no longer supported since April 2021. |
appstream-util shipped with Xenial is so old it doesn't support specific URL types and PNG icons, rendering it useless.
I have ended up changing Dockerfile to use Ubuntu 18.04 as appstream-util from 16.04 was too old to be actually useful. With the current state of PR, you should be able to run |
Hey @dbkr, happy new year! When you've got a minute, can you say if these changes look good to you so far? I want to make sure you are not disagreeing with the general direction I'm taking before I start changing element-builder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some feedback, but all relatively minor: I think this all looks like a sensible direction to be taking!
# used by flathub client \ | ||
apt-get -qq install -y --no-install-recommends flatpak ostree gettext appstream-util curl && \ | ||
apt-get -qq install -y python3 python3-dev python3-pip libgirepository1.0-dev gir1.2-ostree-1.0 libcairo2-dev && \ | ||
curl -sSL https://raw.githubusercontent.com/flatpak/flat-manager/master/flat-manager-client -o /usr/local/bin/flat-manager-client && \ | ||
chmod +x /usr/local/bin/flat-manager-client && \ | ||
pip3 install aiohttp tenacity PyGObject && \ | ||
apt-get purge -y --auto-remove && rm -rf /var/lib/apt/lists/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there not a preexisting docker image for building flatpaks? Looks like the steps are separate, so it doesn't need to be the same docker image that runs electron build.
@@ -0,0 +1,9 @@ | |||
[Desktop Entry] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build
directory is (confusingly) for electron-builder's config, so we should keep things unrelated to electron builder somewhere else. Probably just a top-level flatpak
directory?
@@ -0,0 +1,2 @@ | |||
#!/bin/bash | |||
env TMPDIR="$XDG_RUNTIME_DIR/app/${FLATPAK_ID}" /app/element/element-desktop --no-sandbox "$@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing the --no-sandbox
is due to not being able to run as root? I'm afraid this isn't an option: electron needs this to isolate contexts so this is really important for security.
<id>${FLATPAK_ID}</id> | ||
<name>${FLATPAK_APPNAME}</name> | ||
<project_license>Apache-2.0</project_license> | ||
<developer_name>New Vector Ltd</developer_name> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably wants to be 'Element' (ie. the same as the author in package.json)
<name>${FLATPAK_APPNAME}</name> | ||
<project_license>Apache-2.0</project_license> | ||
<developer_name>New Vector Ltd</developer_name> | ||
<summary>Create, share, communicate, chat and call securely, and bridge to other apps</summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably find some copy for this: I would guess the "Secure and independent communication, connected via Matrix" from the homepage.
<content_attribute id="social-contacts">intense</content_attribute> | ||
</content_rating> | ||
<provides> | ||
<id>im.riot.Riot</id> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we stuck with this from historical precedent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, kind of, keep in mind that this entry basically says "I provide the same capabilities as the app with the identifier im.riot.Riot
" it's not the identifier of this app.
@barthalion When you have the time, can you give some update on what's the current status of this PR? What's left to get the flatpaks to be built? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left suggestions for the vendor name and description
<id>${FLATPAK_ID}</id> | ||
<name>${FLATPAK_APPNAME}</name> | ||
<project_license>Apache-2.0</project_license> | ||
<developer_name>New Vector Ltd</developer_name> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<developer_name>New Vector Ltd</developer_name> | |
<developer_name>Element</developer_name> |
<name>${FLATPAK_APPNAME}</name> | ||
<project_license>Apache-2.0</project_license> | ||
<developer_name>New Vector Ltd</developer_name> | ||
<summary>Create, share, communicate, chat and call securely, and bridge to other apps</summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<summary>Create, share, communicate, chat and call securely, and bridge to other apps</summary> | |
<summary>Secure and independent communication, connected via Matrix</summary> |
Is there any chance this work will be picked up again in the some way? It would be nice for Element to officially be available on Flathub. |
maybe using https://openbuildservice.org/ would make it easier? and it'll enable building for a large distro's/package managers at once, which would be great for official packages for other distro's as well |
|
This change is marked as an internal change (Task), so will not be included in the changelog.