-
Notifications
You must be signed in to change notification settings - Fork 32
Add CI workflow for building the Linux AppImage executable. #140
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
Conversation
2c14275
to
54de24c
Compare
54de24c
to
01b293f
Compare
60d0e75
to
9f5f6d2
Compare
5acb7d1
to
f51ce9d
Compare
a947b02
to
1fd91fe
Compare
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.
So firstly: this works, and that's fantastic and I've seen how much work it took to get to this point - well done!
I have some concerns about maintenance, though. There are a lot of details in here which appear to be very specific to particular versions of dependencies, and where it's not clear to me:
- Why we need to have these things in the Packetry repository.
- Where they came from.
- When, if and how it might be necessary to update them.
Specifically:
appimage/docker/files/etc/passwd
: presumably this is required because the Docker image needs an/etc/passwd
for various tools to function, but there are a lot of what seem like extraneous lines here. Where did these come from, and do we really need all of them? Can we trim this down, or copy most of it from somewhere and just add what we need at build time?appimage/packetry.AppDir/usr/share/doc/*/copyright
: These are presumably Debiancopyright
files - but from what set of package versions, and why those particular packages? I thought at first perhaps it was justlibgtk-4-1
and its dependencies, butlibharfbuzz-dev
is also included. And why are they committed to the Packetry repo, rather than just being copied from the versions used at build time?appimage/action.yml
:- There is a lot of hand-written addition of
.so
symlinks here, using filenames of specific library versions. Can this be made less fragile? I would have thought thatldconfig
with the right parameters could create the symlinks automatically, for instance. - There are hand-coded lists of library dependencies that aren't actually linked to in the current version. But code changes in Packetry or gtk-rs could easily cause some of these to start being linked in as we vary our usage of GTK. Is this something we can automate, with some scripting using
ldd
anddpkg -S
perhaps?
- There is a lot of hand-written addition of
The answer to any or all of these things may be "these things are hardcoded because automating it is more faff than it's worth right now", and that's not unreasonable, but as it stands, if I were looking at needing to make a new release tomorrow without you, I wouldn't know what I needed to do.
So I think if we're not able to automate these details, then we at least need a bit of documentation in the repo that explains how the AppImage build process works, what steps may be needed to update it, and when and how those steps need to be done.
b2dd8e4
to
324fe14
Compare
324fe14
to
c11b024
Compare
Firstly, thank you for a great and gentle review @martinling! :-)
So this is an excellent point. At the time of review this was pulled from the Docker image and modified before adding it to the repo but I've since updated the Dockerfile to just do an in-place edit of the
This was the stickiest issue of all of them. The short version is that the AppImage I talk a bit more about our longer-term options in the maintenance notes. ps. I figured out why libharfbuzz was being pulled in and it's no longer part of the build.
The short version is that
So there are two kinds of lib here:
That said, I think our newly streamlined list no longer has anything left in it that will cause problems if we change our usage of gtk4:
Fair point! I've added some docs to the directory that should help: appimage/README.md In the longer run, when there's a bit more faff available I would like to work on two things:
|
Great, that's a lot tidier!
I'm not sure if I'm understanding correctly. We build GTK4 from source rather than having
Or are we bundling newer versions of those libraries, from a later distro version, when we actually build the AppImage? And if so... how do we know what versions those are, and how do we know whether we still have the correct corresponding license and copyright info for them? As far as I can see, we can't tell. I'm fine with using the But if anything else is ending up in the AppImage, then that's a moving target which can become out of sync with what we have in the Packetry repo, at any time, and without any warning.
Joy is duly sparked, thank you.
Why are these pulled in at all? Is this due to the difference in dependencies between the minimal GTK we build from source, and that of the distro packages?
These are supposed to be dealt with by the AppImage
I'm fine with hardcoding this list if all of our dependencies are hardcoded to fixed versions, and if CI will fail in some way if we get out of sync. Is that the case, though?
These are very helpful, thank you! I think I'm still not fully understanding though, as is probably clear from the above.
I'm fine with tracking upstream license changes by hand, if we're also updating dependency versions by hand. But if we're going to have a situation where the binaries can get out of sync with the license/copyright text, just because a distro shipped new packages and the CI picked them up, that's not really OK.
Is the original beyond improvement? |
Ah, now I understand the confusion. So all of those are generated by the gtk4 build. This means they will always correspond to the release version of the sources we're compiling. This confused the hell out of me as well because at one point we were pulling libharfbuzz from the buster packages because it got pulled in by some other utterly random dependency that we also didn't need but the gtk build scripts die if it's not there. This was causing a whole bunch of other stuff to not be built from gtk4 sources and then getting pulled into the AppImage from buster that should have been supplied by the sources. So to answer your first question:
We're only extracting and copying the This is why I think we're okay with the current approach.
Nope, nothing else from upstream ending up in the AppImage except the So I reckon we're good?
Yeah exactly that, in a non
All dependencies are hardcoded to fixed versions. Unless we change: a) the gtk release version we're building from or …it will keep running until the day that CI breaks because our gtk4 release is too old to compile packetry anymore. 🫠
👌
💯
It's more a case of what I'd like it (and in some cases Having now spent a couple of weeks working with them both I think there's a strong case to be made for keeping the tool surface area as narrow as possible. Both those tools are (understandably) making very broad assumptions that we then have to undo as part of our workflow. It would probably be a lot cleaner and more maintenance-friendly in the long run to just generate |
Ahh, OK! I hadn't realised that the GTK4 tarball build would pull those other projects in directly. Please add something about this to the docs! 😄 The other detail that had made this confusing for me was that the I've now figured out that it's this step: - name: Run build appimage action (Linux)
uses: ./appimage/ ...and that the
Yes, I agree that's fine.
Yup, I don't see a need to try to streamline this right now. |
Done 😆
Done, good call! |
This PR adds support for building the Linux AppImage binary during CI workflow.
Overview
The workflow file is .github/workflows/appimage.yml and is structured as follows:
Step 1: Checks if the docker container exists or needs to be updated, if not, go to Step 3.
Step 2: Builds the appimage/docker/Dockerfile for compiling packetry and creating an appimage.
Step 3: Runs pretty much the same workflow as the
rust.yml
workflow to compile & test packetry inside the docker container.Step 4: Invokes a custom action appimage/action.yml to build the Linux AppImage binary.
Step 5: Uploads the Linux AppImage binary artifact.
Docker Image Notes
GTK_VERSION
sets the version of GTK sources to be downloaded and built.NODE_VERSION
sets the version of node to install for the use ofSwatinem/rust-cache@v2
USER root
but it turns out that was wasted becauseGitHub Actions does not support containers running as anything but root:
$HOME
for the root user with/github/home
which of course causes all thethings to fail.
/github/home
in your container, GitHub actions will happily delete the contents./etc/passwd
to changeroot
home dir to/github/home
during the container build and then copying everything you need into$HOME
before running the workflow.<repo>-gtk-<gtk-version>:<branch>
so we don't run into issues when working on branches or PR's.Outstanding issues
Closes #117