-
-
Notifications
You must be signed in to change notification settings - Fork 32
feat(installer): initial flatpak packaging #223
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
base: main
Are you sure you want to change the base?
Conversation
@craftablescience do you have any time to review and merge? |
Sorry for the holdup, there's a lot going on in my life right now |
I have other tweaks I'll be pushing, but I don't know what the flatpak builder wants from the desktop entry. Will merge as soon as I get clarification |
Sorry to hassle you, its not that important in the grand scheme of things, I had just worried that you hadn't seen it. Take as long as you need |
ad5dc9c
to
3153256
Compare
1db228b
to
c483806
Compare
Hi @lina-bh, These changes were a bit too bleeding edge for me to be comfortable merging before last release, so I waited to work on this until that was out. Please let me know if these changes work, I haven't tested them but in theory these changes revert the Linux installer to how it was before, and the flatpak should still work the same. If this works for you, I'm good to merge it in its current state. |
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.
Overall, there's a couple of more things to take a look at.
I think it would be better to unify .desktop and other things because its not flatpak related and others package would also benefit, but it can be break current packages.
I don't know if its weird someone from nowhere reviewing but a time ago i was trying to make a flatpak package too for vpkedit. I had some trouble because i don't know C++ nor CMake and with a lot of updates being pushed there was a moment where i couldn't get it working so i stopped, but i'm gladly to see someone is doing, so i'm here to try to help.
@@ -0,0 +1,28 @@ | |||
"$schema": "https://raw.githubusercontent.com/flatpak/flatpak-builder/main/data/flatpak-manifest.schema.json" | |||
id: info.craftablescience.vpkedit.Devel |
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.
Wouldn't be better using the name capitalized? info.craftablescience.VPKEdit
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.
uppercased rDNS names are harder to tab complete which upsets me to no end
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.
it's more correct but ultimately doesn't matter, up to you or craftablescience
finish-args: | ||
- "--socket=wayland" | ||
- "--socket=fallback-x11" | ||
- "--filesystem=host" | ||
- "--env=QT_QPA_PLATFORMTHEME=kde" |
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 know the current model of flatpak and portals doesn't work much in favor of how vpkedit gets split vpk, but i don't think it's a good idea to have access to host
. I would prefer something like home
but still not ideal, and for certain cases it wouldn't work ootb but for now its fine.
I don't think QT_QPA_PLATFORMTHEME=kde
is needed and if this is gonna be on flathub it probably will be call out.
Other than that there's a couple of missing args like --device=dri
and --share=ipc
which is generally used.
build-args: | ||
- "--share=network" |
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.
Following about being on flathub, you cannot build with network permission, since this is a development manifest there's no problem but just saying. Also, vpkedit even needs internet to build?
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.
one of the dependencies git-clones another dependency in a certified C++ build systems moment
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.
oh i see, that's annoying but we should be able to workaround downloading ahead of time and placing where its expect to be hopefully
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.
its all using cmake's FetchContent, so if the repos are precloned in the places it expects to find them it should build without internet
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 thought it was only one dependency that require this but found out it isn't and also couldn't figure it out where to put it to the build process automatically use instead of trying to download
but well, i continued to do stuff and just enabled network in the meantime, but then one of the dependencies i can't download and now tbh i don't know what to do
fatal: unable to access 'https://sourceware.org/git/bzip2.git/': Could not resolve host: sourceware.org
sourcepp
seems to be a little finicky to build in flatpak and would require a lot of maintenance to keep everything updated, so instead of trying to compile, would be good to just download a compiled version? this would need help to distribute the library and to use in the app which i don't know enough to it myself
another option i see is just skipping building in flatpak and just extracting the release packages, far from ideal but it should work
any thoughts?
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 think hosting precompiled binaries that the flatpak links to is a workable idea. If VPKEdit is changed to use these binaries for every platform, it would also save a lot of time building locally and in CI. I'll need to do some setup on the sourcepp end to make an auto-release button first ofc.
I was planning to set up releases at some point regardless to distribute the C wrapper dlls without the end user needing to compile the library.
Of course on the flip side I also think that building the VPKEdit flatpak without sourcepp prebuilt is also workable and might even be easier once the FetchContent stuff is resolved. That error you're getting with bzip2 might be because it's cloning multiple copies at the same time? Or sourceforge is just unreliable... if its unsolvable there is a CMake option to disable bzip2 compression support.
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.
Perhaps it's possible to build it as a module inside the KDE SDK for the flatpak builder and then bring it into the build afterwards? This would mean that there is no ABI drift
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.
That error you're getting with bzip2 might be because it's cloning multiple copies at the same time? Or sourceforge is just unreliable... if its unsolvable there is a CMake option to disable bzip2 compression support.
i think you mean sourceware.org
(which i never heard before) but i really can't access the domain at all for some reason, wonder what could break without bzip2 or any other dependency on the list
Perhaps it's possible to build it as a module inside the KDE SDK for the flatpak builder and then bring it into the build afterwards? This would mean that there is no ABI drift
i don't think i'm following what you saying, can explain wdym?
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.
One of the benefits of containerisation is having a consistent set of libraries to build against, getting around the DLL Hell that mixing and matching ABIs and libraries from different distributions can cause.
This is why shipping Qt shared objects inside tarballs for Linux apps kind of gives me the willies. We're not Windows, we don't really have stable ABI guarantees since we got used to being able to rebuild things at will, usually by our distribution, but Flatpak is a concession after learning that doesn't scale.
It should be possible to create an artifact that contains the binaries for the dependencies that ask for network access, then import that artifact into a sandboxed build for Flathub, without the build system for VPKEdit needing network access. All the while matching the SDK environment so that everything is consistent.
sourceware.org
is Red Hat (ex-Cygnus Solutions). It appears the HTTP redirect is broken for the site though
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.
Yeah but i just ended up got it working after a little help and inspiration from the nix package, also there's the detail that compiling together helps to compile for another architectures.
About the sourceware.org
, i was using cloudflare dns which for some reason couldn't access so i just changed.
Extrapolated a bit but now only has some more things to do like metainfo and testing to make sure everything works.
- "-DVPKEDIT_USE_LTO=ON" | ||
- "-DVPKEDIT_BUILD_INSTALLER=OFF" | ||
- "-DVPKEDIT_BUILD_FLATPAK=ON" | ||
- "-DVPKEDIT_FLATPAK_ID=info.craftablescience.vpkedit.Devel" |
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.
You don't really need to specify, you can just use the env FLATPAK_ID
internally.
flatpak: | ||
cd $(_GIT_TOPLEVEL) && \ | ||
flatpak-builder $(FLATPAK_BUILDER_FLAGS) $(FLATPAK_BUILD_DIR) $(FLATPAK_BUILD_MANIFEST) || \ | ||
printf "you may need to add flathub to your target installation\n\ | ||
flatpak remote-add --if-not-exists %s flathub https://dl.flathub.org/repo/flathub.flatpakrepo\n" \ | ||
$(FLATPAK_INSTALLATION) \ | ||
2>&1 |
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.
Doesn't have the option to use the flatpak version of flatpak-builder
(org.flatpak.Builder
) and the way it seems to work is that any error that could occur during the compilation would also print about needing to add flathub.
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.
using org.flatpak.Builder
is an obvious change yeah, i wrote this on ublue where it is installed on the host
Been a while but i think this PR and trunk have diverged, please take a look at main. As it happens life decided to start handing me my arse on a daily basis so I haven't gotten around to looking at this again
…On Mon, 31 Mar 2025, at 06:04, underscorejoser wrote:
***@***.**** commented on this pull request.
Overall, there's a couple of more things to take a look at.
I think it would be better to unify .desktop and other things because its not flatpak related and others package would also benefit, but it can be break current packages.
I don't know if its weird someone from nowhere reviewing but a time ago i was trying to make a flatpak package too for vpkedit. I had some trouble because i don't know C++ nor CMake and with a lot of updates being pushed there was a moment where i couldn't get it working so i stopped, but i'm gladly to see someone is doing, so i'm here to try to help.
In src/installer/flatpak/info.craftablescience.vpkedit.Devel.yaml <#223 (comment)>:
> @@ -0,0 +1,28 @@
+"$schema": "https://raw.githubusercontent.com/flatpak/flatpak-builder/main/data/flatpak-manifest.schema.json"
+id: info.craftablescience.vpkedit.Devel
Wouldn't be better using the name capitalized? `info.craftablescience.VPKEdit`
In src/installer/flatpak/info.craftablescience.vpkedit.Devel.yaml <#223 (comment)>:
> +finish-args:
+ - "--socket=wayland"
+ - "--socket=fallback-x11"
+ - "--filesystem=host"
+ - "--env=QT_QPA_PLATFORMTHEME=kde"
I know the current model of flatpak and portals doesn't work much in favor of how vpkedit gets split vpk, but i don't think it's a good idea to have access to `host`. I would prefer something like `home` but still not ideal, and for certain cases it wouldn't work ootb but for now its fine.
I don't think `QT_QPA_PLATFORMTHEME=kde` is needed and if this is gonna be on flathub it probably will be call out.
Other than that there's a couple of missing args like `--device=dri` and `--share=ipc` which is generally used.
In src/installer/flatpak/info.craftablescience.vpkedit.Devel.yaml <#223 (comment)>:
> + build-args:
+ - "--share=network"
Following about being on flathub, you cannot build with network permission, since this is a development manifest there's no problem but just saying. Also, vpkedit even needs internet to build?
In src/installer/flatpak/info.craftablescience.vpkedit.Devel.yaml <#223 (comment)>:
> + - "--socket=fallback-x11"
+ - "--filesystem=host"
+ - "--env=QT_QPA_PLATFORMTHEME=kde"
+modules:
+ - name: vpkedit
+ buildsystem: cmake-ninja
+ build-options:
+ build-args:
+ - "--share=network"
+ config-opts:
+ - "-Wno-dev"
+ - "-DCMAKE_BUILD_TYPE=Release"
+ - "-DVPKEDIT_USE_LTO=ON"
+ - "-DVPKEDIT_BUILD_INSTALLER=OFF"
+ - "-DVPKEDIT_BUILD_FLATPAK=ON"
+ - "-DVPKEDIT_FLATPAK_ID=info.craftablescience.vpkedit.Devel"
You don't really need to specify, you can just use the env `FLATPAK_ID` internally.
In src/installer/flatpak/Makefile <#223 (comment)>:
> +flatpak:
+ cd $(_GIT_TOPLEVEL) && \
+ flatpak-builder $(FLATPAK_BUILDER_FLAGS) $(FLATPAK_BUILD_DIR) $(FLATPAK_BUILD_MANIFEST) || \
+ printf "you may need to add flathub to your target installation\n\
+ flatpak remote-add --if-not-exists %s flathub https://dl.flathub.org/repo/flathub.flatpakrepo\n" \
+ $(FLATPAK_INSTALLATION) \
+ 2>&1
Doesn't have the option to use the flatpak version of `flatpak-builder` (`org.flatpak.Builder`) and the way it seems to work is that any error that could occur during the compilation would also print about needing to add flathub.
—
Reply to this email directly, view it on GitHub <#223 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIYEFFV7DIPO752Y3YYBXM32XDEHVAVCNFSM6AAAAABXTDPAJSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMRYGIZDCNBVGI>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Would you prefer i take your work to another pull request superseding it? |
absolutely no problem. tbf I had written this to get it working at any level for atomic systems but I am on regular r/w root now. if you would like to finish the long tail for flathub you have no objection from me 😅 I do wish that flatpak could pull packages from a bog standard OCI registry, but it appears that it's harder than it looks because you need to hand flatpak some metadata that you can't easily get working for eg GitHub container registry |
Discussion in #221
What works:
What currently does not work:
--filesystem=host
which may weird out some usersPotentially controversial:
What was not tested:
What is currently missing but would be nice:
I'd like to get the current work into the repo so that it can be built upon further
How to build and install:
src/installer/linux
, runmake flatpak
to build andmake flatpak-install
to install to your current systemFLATPAK_INSTALLATION
: change this to--user
if, for example, you are running inside a development containerFLATPAK_BUILDER_FLAGS
/FLATPAK_INSTALL_FLAGS
: override flags passed toflatpak-builder
andflatpak install
FLATPAK_BUILDER_EXTRA_FLAGS
: as above, but in addition to the default flags