feat: build the Linux AppImage in a container on any distribution #1936

Merged
jedimastermaniac merged 2 commits from feat/linux-docker-build into main 2026-11-12 03:03:47 +00:00
jedimastermaniac commented 2026-11-10 21:08:26 +00:00 (Migrated from github.com)

scripts/linux/deps.sh and appimage.sh only make a working AppImage on Ubuntu 22.04, the system CI builds it in. appimage.sh copies the build system's C library and C++ headers into the AppImage as the sysroot the player's game is built against, so a build on a newer distribution (Arch, Fedora, a current Ubuntu) makes an AppImage, and a game, that need that system's newer glibc and do not run on the glibc 2.35 systems a release is for. Contributors off Ubuntu 22.04 had no supported way to build the AppImage locally.

scripts/linux/Dockerfile is CI's environment: ubuntu:22.04 with the linux-appimage workflow's packages and toolchain PPA. scripts/linux/ docker.sh builds it and runs deps.sh and appimage.sh with the checkout mounted, as the caller's user under Docker, rootless Docker and Podman, so build-linux/ stays the caller's. The AppImage lands in build-linux/ as before; deps.sh and appimage.sh are unchanged.

The container keeps its own build-linux/docker-deps and docker-vk-work. deps.sh keeps any part already built and appimage.sh only relinks the Vulkan back end's objects, so a host build's were reused: the first container build linked Arch-built objects and hookshot_vk.so failed with 8 unresolved symbols (__isoc23_strtol, std::format). With them apart, hookshot_vk.so needs GLIBC_2.35 at most and check_player_build.sh passes.

tests/test_linux_docker_packages.py checks that the Dockerfile's base image, packages and PPA stay the workflow's. CONTRIBUTING.md's Linux section says when and how to use the container.

Assisted-by: Claude Code

scripts/linux/deps.sh and appimage.sh only make a working AppImage on Ubuntu 22.04, the system CI builds it in. appimage.sh copies the build system's C library and C++ headers into the AppImage as the sysroot the player's game is built against, so a build on a newer distribution (Arch, Fedora, a current Ubuntu) makes an AppImage, and a game, that need that system's newer glibc and do not run on the glibc 2.35 systems a release is for. Contributors off Ubuntu 22.04 had no supported way to build the AppImage locally. scripts/linux/Dockerfile is CI's environment: ubuntu:22.04 with the linux-appimage workflow's packages and toolchain PPA. scripts/linux/ docker.sh builds it and runs deps.sh and appimage.sh with the checkout mounted, as the caller's user under Docker, rootless Docker and Podman, so build-linux/ stays the caller's. The AppImage lands in build-linux/ as before; deps.sh and appimage.sh are unchanged. The container keeps its own build-linux/docker-deps and docker-vk-work. deps.sh keeps any part already built and appimage.sh only relinks the Vulkan back end's objects, so a host build's were reused: the first container build linked Arch-built objects and hookshot_vk.so failed with 8 unresolved symbols (__isoc23_strtol, std::format). With them apart, hookshot_vk.so needs GLIBC_2.35 at most and check_player_build.sh passes. tests/test_linux_docker_packages.py checks that the Dockerfile's base image, packages and PPA stay the workflow's. CONTRIBUTING.md's Linux section says when and how to use the container. Assisted-by: Claude Code
hckrman101 (Migrated from github.com) reviewed 2026-11-11 02:18:37 +00:00
hckrman101 (Migrated from github.com) left a comment

Nice, this fills a real gap: building the AppImage off Ubuntu 22.04 silently gives a glibc-too-new sysroot. I read the Dockerfile, docker.sh and the test against the workflow's package step and against deps.sh/appimage.sh on this head. The package list, PPA and base image match the workflow, the regexes in the test do pick up both lists (I traced the line-continuation joining and the ;/& terminators), and mounting docker-vk-work over build-linux/vk-work lines up with VKW=$B/vk-work in appimage.sh (it only rm -fs the .so inside, so the mount point is safe). DEPS is honoured by both scripts. I could not build or run it, so the container run itself is untested on my side.

One thing worth handling, since Fedora is named in the description: on an SELinux-enforcing host (Fedora, RHEL) a plain -v "$PWD:/src" is denied inside the container, for Docker and Podman alike. Adding --security-opt label=disable (or a :z suffix on the mounts) when /sys/fs/selinux/enforce reads 1 would avoid a confusing "permission denied" on the first run.

Nits: the header comment in docker.sh has one very long line (the DOCKER=podman sentence), worth rewrapping to match the rest of the file. The Dockerfile's last CMD repeats docker.sh's default command, fine, just two places to keep in step.

CI has not run on this head (all three workflows are action_required, waiting for approval to run for a first-time contributor), so nothing to go on there yet. Decision on merging stays with the maintainer; I found nothing blocking in the code.

Nice, this fills a real gap: building the AppImage off Ubuntu 22.04 silently gives a glibc-too-new sysroot. I read the Dockerfile, docker.sh and the test against the workflow's package step and against deps.sh/appimage.sh on this head. The package list, PPA and base image match the workflow, the regexes in the test do pick up both lists (I traced the line-continuation joining and the `;`/`&` terminators), and mounting docker-vk-work over build-linux/vk-work lines up with `VKW=$B/vk-work` in appimage.sh (it only `rm -f`s the .so inside, so the mount point is safe). `DEPS` is honoured by both scripts. I could not build or run it, so the container run itself is untested on my side. One thing worth handling, since Fedora is named in the description: on an SELinux-enforcing host (Fedora, RHEL) a plain `-v "$PWD:/src"` is denied inside the container, for Docker and Podman alike. Adding `--security-opt label=disable` (or a `:z` suffix on the mounts) when `/sys/fs/selinux/enforce` reads 1 would avoid a confusing "permission denied" on the first run. Nits: the header comment in docker.sh has one very long line (the DOCKER=podman sentence), worth rewrapping to match the rest of the file. The Dockerfile's last `CMD` repeats docker.sh's default command, fine, just two places to keep in step. CI has not run on this head (all three workflows are `action_required`, waiting for approval to run for a first-time contributor), so nothing to go on there yet. Decision on merging stays with the maintainer; I found nothing blocking in the code.
jedimastermaniac commented 2026-11-11 11:13:18 +00:00 (Migrated from github.com)

Loaded up qemu with a fedora cloud image and i confirm your comments and behavior
both podman and docker (with moby-engine) reproduce the case in comment
Fixed and runs on fedora now where /sys/fs/selinux/enforce == 1

Loaded up qemu with a fedora cloud image and i confirm your comments and behavior both podman and docker (with moby-engine) reproduce the case in comment Fixed and runs on fedora now where `/sys/fs/selinux/enforce` == `1`
Joey-Magnus (Migrated from github.com) requested changes 2026-11-11 21:46:18 +00:00
Joey-Magnus (Migrated from github.com) left a comment

The SELinux fix in 1f084b9 looks right: label=disable only when /sys/fs/selinux/enforce reads 1, and no :z relabelling of the checkout. I merged the head into current main and ran the PR's own test. tests/test_linux_docker_packages.py fails in test_ppas:

Items in the first set but not the second: 'ppa:ubuntu-toolchain-r/test'; in the second but not the first: '--no-update'

#2275 changed the workflow's step to add-apt-repository -y --no-update ppa:ubuntu-toolchain-r/test (.github/workflows/linux-appimage.yml:78). ppas() only allows an optional -y, so it now captures the flag as the PPA. Skipping any leading -... options in the regex would fix it, and that keeps the check as strict as it is now. The package-list half still matches the workflow. Please rebase onto main so this shows up in CI, and so CI runs at all. The test workflows haven't run on this PR yet.

hckrman101's nit about the long header line in docker.sh is still open.

The SELinux fix in 1f084b9 looks right: `label=disable` only when `/sys/fs/selinux/enforce` reads 1, and no `:z` relabelling of the checkout. I merged the head into current main and ran the PR's own test. `tests/test_linux_docker_packages.py` fails in `test_ppas`: `Items in the first set but not the second: 'ppa:ubuntu-toolchain-r/test'; in the second but not the first: '--no-update'` #2275 changed the workflow's step to `add-apt-repository -y --no-update ppa:ubuntu-toolchain-r/test` (.github/workflows/linux-appimage.yml:78). `ppas()` only allows an optional `-y`, so it now captures the flag as the PPA. Skipping any leading `-...` options in the regex would fix it, and that keeps the check as strict as it is now. The package-list half still matches the workflow. Please rebase onto main so this shows up in CI, and so CI runs at all. The test workflows haven't run on this PR yet. hckrman101's nit about the long header line in docker.sh is still open.
hckrman101 (Migrated from github.com) reviewed 2026-11-11 22:21:00 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for the SELinux fix: label=disable only when /sys/fs/selinux/enforce reads 1, no :z relabelling of the checkout, and the CONTRIBUTING note explains why. That point is resolved, and your Fedora VM check covers it.

Still open:

  • Joey-Magnus's finding is right, and I checked it against main. The workflow now runs add-apt-repository -y --no-update ppa:ubuntu-toolchain-r/test (#2275), and ppas() only allows an optional -y, so it captures --no-update as the PPA and test_ppas fails. Skipping any leading - options in the regex fixes it (inline note). The package half is unaffected; installs() already drops - arguments.
  • The long header line in docker.sh (the DOCKER=podman sentence, about 200 characters) is still unwrapped. Nit.

Please rebase onto main so the test workflows run. CI only has the title check so far, and the test suite is not running on this PR. I could not build or run the container myself. The decision to merge stays with the maintainer; the test fix is the only blocker I see.

Thanks for the SELinux fix: `label=disable` only when `/sys/fs/selinux/enforce` reads 1, no `:z` relabelling of the checkout, and the CONTRIBUTING note explains why. That point is resolved, and your Fedora VM check covers it. Still open: - Joey-Magnus's finding is right, and I checked it against main. The workflow now runs `add-apt-repository -y --no-update ppa:ubuntu-toolchain-r/test` (#2275), and `ppas()` only allows an optional `-y`, so it captures `--no-update` as the PPA and `test_ppas` fails. Skipping any leading `-` options in the regex fixes it (inline note). The package half is unaffected; `installs()` already drops `-` arguments. - The long header line in docker.sh (the DOCKER=podman sentence, about 200 characters) is still unwrapped. Nit. Please rebase onto main so the test workflows run. CI only has the title check so far, and the test suite is not running on this PR. I could not build or run the container myself. The decision to merge stays with the maintainer; the test fix is the only blocker I see.
@ -0,0 +26,4 @@
def ppas(text):
return set(re.findall(r"add-apt-repository\s+(?:-y\s+)?(\S+)", text))
hckrman101 (Migrated from github.com) commented 2026-11-11 22:21:00 +00:00

Fails against current main: the workflow's line is add-apt-repository -y --no-update ppa:..., so (\S+) captures --no-update. Something like r"add-apt-repository(?:\s+-\S+)*\s+(\S+)" skips any options and still keeps the check strict.

Fails against current main: the workflow's line is `add-apt-repository -y --no-update ppa:...`, so `(\S+)` captures `--no-update`. Something like `r"add-apt-repository(?:\s+-\S+)*\s+(\S+)"` skips any options and still keeps the check strict.
Commenting is not possible because the repository is archived.
No description provided.