Sorting out yet another AppImage breakage. #1947
Conversation
WalkthroughThe Linux CI workflow was updated to allow container access to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tools/build/Dockerfile (1)
51-53: Collapse the threeRUNlayers, pin version & add checksum forappimage-builder.Multiple layers slow the build and, again, an unchecked binary is fetched.
-RUN wget -O appimage-builder-x86_64.AppImage https://github.com/AppImageCrafters/appimage-builder/releases/download/v1.1.0/appimage-builder-1.1.0-x86_64.AppImage -RUN chmod +x appimage-builder-x86_64.AppImage -RUN mv appimage-builder-x86_64.AppImage /usr/local/bin/appimage-builder +RUN curl -L -o /usr/local/bin/appimage-builder \ + https://github.com/AppImageCrafters/appimage-builder/releases/download/v1.1.0/appimage-builder-1.1.0-x86_64.AppImage && \ + echo "SHASUM /usr/local/bin/appimage-builder" | sha256sum -c - && \ + chmod +x /usr/local/bin/appimage-builderAgain, replace
SHASUMwith the official checksum.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/linux-build.yml(1 hunks)tools/build/Dockerfile(1 hunks)tools/build/appimage-dpkg.patch(0 hunks)
💤 Files with no reviewable changes (1)
- tools/build/appimage-dpkg.patch
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: pcsx-redux (x86_64-linux)
- GitHub Check: pcsx-redux (aarch64-linux)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
- GitHub Check: macos-build-and-test-toolchain
- GitHub Check: aur-build
- GitHub Check: asan
- GitHub Check: cross-arm64
- GitHub Check: build-openbios
- GitHub Check: coverage
- GitHub Check: toolchain
tools/build/Dockerfile
Outdated
| # AppImage dependencies | ||
| RUN apt install -y libfuse2t64 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding fuse & required capabilities, not only the userspace library.
libfuse2t64 provides the userspace library, but AppImages still need the FUSE kernel module and the fusermount helper.
On Ubuntu 24.04 those come from the fuse package. In addition, mounting inside the container requires the SYS_ADMIN capability (or --privileged) in the runtime options. Only adding /dev/fuse will still lead to EPERM at mount-time.
Tagging so this does not regress CI again.
🤖 Prompt for AI Agents
In tools/build/Dockerfile around lines 40 to 42, the Dockerfile installs only
the userspace library libfuse2t64, but it also needs the fuse package to provide
the FUSE kernel module and fusermount helper. Update the Dockerfile to install
the fuse package alongside libfuse2t64. Additionally, ensure that the container
runtime is configured with the SYS_ADMIN capability or run with --privileged to
allow mounting, as adding /dev/fuse alone will cause permission errors.
| RUN wget https://github.com/AppImage/appimagetool/releases/download/continuous/appimagetool-x86_64.AppImage && \ | ||
| chmod +x /tmp/appimagetool-x86_64.AppImage && \ | ||
| cd /opt && /tmp/appimagetool-x86_64.AppImage --appimage-extract && \ | ||
| mv squashfs-root appimage-tool.AppDir && \ | ||
| ln -s /opt/appimage-tool.AppDir/AppRun /usr/bin/appimagetool && \ | ||
| rm /tmp/appimagetool-x86_64.AppImage |
There was a problem hiding this comment.
Pin the download & verify checksum of appimagetool for supply-chain safety.
The build now pulls appimagetool from the continuous channel each time.
That URL always points to the latest artefact, so the resulting image is no longer reproducible and the hash is not verified.
-RUN wget https://github.com/AppImage/appimagetool/releases/download/continuous/appimagetool-x86_64.AppImage && \
+RUN curl -L -o appimagetool-x86_64.AppImage \
+ https://github.com/AppImage/appimagetool/releases/download/continuous/appimagetool-x86_64.AppImage && \
+ echo "SHASUM appimagetool-x86_64.AppImage" | sha256sum -c - && \Replace SHASUM with the published hash (or a pinned tag instead of continuous).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tools/build/Dockerfile around lines 44 to 49, the appimagetool is downloaded
from the continuous channel without pinning to a specific version or verifying
its checksum, which compromises build reproducibility and supply-chain security.
To fix this, replace the URL to download a specific tagged release version of
appimagetool instead of continuous, and add a step to verify the downloaded
file's checksum against the published hash before proceeding with extraction and
installation.
nicolasnoble
force-pushed
the
appimage-bullshit
branch
from
24629ca to
e5244ae
Compare
AppImage/AppImageKit#1395