Skip to content

Don't bundle alsa lib #1

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.

Sign up for GitHub

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

Open
jonaski wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Don't bundle alsa lib #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ The AppDir can either be produced by a single application's own build (e.g. Stra
| GStreamer 1.0 | `libgstreamer-1.0.so` | The gstreamer-1.0 plugin directory (filtered - see [GStreamer plugin selection](#gstreamer-plugin-selection)) plus `gst-plugin-scanner`, plus `libsoup-3.0.so.0` if `libgstsoup.so` is present (it's `dlopen()`'d, not linked, so the ordinary dependency walk misses it) |
| Gdk-Pixbuf | `libgdk_pixbuf` | Pixbuf loaders + a patched `loaders.cache` |
| GTK 2/3/4 | `libgtk-<2\|3\|4>` | The matching GTK module directory, default theme (GTK ≤ 3), and a patched `immodules.cache` (GTK ≤ 3) |
| ALSA | `libasound` | The `alsa-lib` plugin directory |
| ALSA | `libasound` | **Never bundled** - `libasound.so*` is removed from the dependency set so it always resolves against the running system's alsa-lib (plugins, config, and hardware UCM profiles are versioned together and bundling them breaks device access on a mismatched host) |

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== README excerpt ==\n'
nl -ba README.md | sed -n '25,40p'

printf '\n== data/apprun.sh outline ==\n'
ast-grep outline data/apprun.sh --view expanded

printf '\n== resolver-related matches ==\n'
rg -n "libasound|alsa|LD_LIBRARY_PATH|rpath|resolve|library|dlopen|ldconfig|/usr/lib|/lib" data/apprun.sh README.md

Repository: jonaski/appimagetools

Length of output: 220

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== data/apprun.sh full relevant sections ==\n'
nl -ba data/apprun.sh | sed -n '1,260p'

Repository: jonaski/appimagetools

Length of output: 243

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== README excerpt ==\n'
cat -n README.md | sed -n '25,40p'

printf '\n== data/apprun.sh status ==\n'
if [ -f data/apprun.sh ]; then
  printf 'exists\n'
  printf '\n== data/apprun.sh excerpt ==\n'
  cat -n data/apprun.sh | sed -n '1,260p'
else
  printf 'missing\n'
fi

printf '\n== search for ALSA-related logic ==\n'
rg -n "libasound|alsa|LD_LIBRARY_PATH|rpath|resolve|library|dlopen|ldconfig|/usr/lib|/lib" README.md data || true

Repository: jonaski/appimagetools

Length of output: 13645

Avoid claiming unconditional ALSA resolution — data/apprun.sh only preloads libasound.so.2 from a fixed set of paths, so hosts that install ALSA elsewhere won’t get that guarantee.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` at line 33, Update the ALSA entry in README.md to remove the
unconditional “always resolves” claim and accurately describe the behavior of
data/apprun.sh: libasound.so.2 is preloaded only from its configured fixed
paths, while other host installations may not be resolved by that mechanism.

| PulseAudio | `libpulse` | The PulseAudio plugin directory |
| GIO | `libgio-2.0` | `gio/modules/` + `giomodule.cache`, exposed via `GIO_EXTRA_MODULES` (see [GIO modules and the GnuTLS trust store](#gio-modules-and-the-gnutls-trust-store)) |
| GLib schemas | `usr/share/glib-2.0/schemas` present in the AppDir | Compiles them with `glib-compile-schemas` if not already compiled |
Expand Down
11 changes: 11 additions & 0 deletions data/apprun.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ if [ -e "${ROOT}/etc/crypto-policies/back-ends/gnutls.config" ]; then
fi


# ALSA (libasound.so.2) is deliberately never bundled (see MiscDeploy::HandleAlsa): its plugin modules, config, and hardware UCM profiles are versioned together by the running distro, and bundling a build-time snapshot instead breaks device access on a mismatched host.
# But the bundled ld-linux has had "/lib", "/usr", and "/etc" patched out of its own compiled-in strings (see DeployInterpreter) so it can no longer fall back to the default system library directories or /etc/ld.so.cache to find anything not explicitly rpath'd within the AppDir.
# Preloading the host's copy by its exact absolute path satisfies that one SONAME directly for the whole process, without reopening host-library search (and the ABI mismatches that guards against) for anything else that *is* bundled.
for ALSA_LIB in /usr/lib/x86_64-linux-gnu/libasound.so.2 /usr/lib/aarch64-linux-gnu/libasound.so.2 /usr/lib64/libasound.so.2 /usr/lib/libasound.so.2 /lib/x86_64-linux-gnu/libasound.so.2; do
if [ -e "${ALSA_LIB}" ]; then
export LD_PRELOAD="${LD_PRELOAD:+${LD_PRELOAD}:}${ALSA_LIB}"
break
fi
done
Comment on lines +75 to +80

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- data/apprun.sh ---\n'
nl -ba data/apprun.sh | sed -n '1,160p'

printf '\n--- src/dependencywalker.h ---\n'
nl -ba src/dependencywalker.h | sed -n '1,140p'

printf '\n--- src/miscdeploy.cpp ---\n'
nl -ba src/miscdeploy.cpp | sed -n '1,260p'

printf '\n--- src/miscdeploy.h ---\n'
nl -ba src/miscdeploy.h | sed -n '1,120p'

printf '\n--- README.md ---\n'
nl -ba README.md | sed -n '1,120p'

printf '\n--- ALSA search ---\n'
rg -n "libasound|ALSA|alsa-lib|AppRun|LD_PRELOAD" data src README.md

Repository: jonaski/appimagetools

Length of output: 250

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- data/apprun.sh ---'
nl -ba data/apprun.sh | sed -n '1,160p'

echo
echo '--- src/dependencywalker.h ---'
nl -ba src/dependencywalker.h | sed -n '1,140p'

echo
echo '--- src/miscdeploy.cpp ---'
nl -ba src/miscdeploy.cpp | sed -n '1,260p'

echo
echo '--- src/miscdeploy.h ---'
nl -ba src/miscdeploy.h | sed -n '1,120p'

echo
echo '--- README.md ---'
nl -ba README.md | sed -n '1,120p'

echo
echo '--- ALSA search ---'
rg -n "libasound|ALSA|alsa-lib|AppRun|LD_PRELOAD" data src README.md

Repository: jonaski/appimagetools

Length of output: 222

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- data/apprun.sh ---'
sed -n '1,160p' data/apprun.sh | cat -n

echo
echo '--- src/dependencywalker.h ---'
sed -n '1,140p' src/dependencywalker.h | cat -n

echo
echo '--- src/miscdeploy.cpp ---'
sed -n '1,260p' src/miscdeploy.cpp | cat -n

echo
echo '--- src/miscdeploy.h ---'
sed -n '1,120p' src/miscdeploy.h | cat -n

echo
echo '--- README.md ---'
sed -n '1,120p' README.md | cat -n

echo
echo '--- ALSA search ---'
rg -n "libasound|ALSA|alsa-lib|AppRun|LD_PRELOAD" data src README.md

Repository: jonaski/appimagetools

Length of output: 50379

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- arch/support search ---'
rg -n "x86_64|aarch64|armhf|arm64|riscv64|ppc64le|supported architectures|architecture" README.md src data | head -n 200

echo
echo '--- libasound search ---'
rg -n "libasound|ALSA" src data README.md

echo
echo '--- AppRun template source ---'
sed -n '40,90p' src/appimagedeploy.cpp | cat -n

Repository: jonaski/appimagetools

Length of output: 7088

AppRun only preloads ALSA from a fixed path list, so the “always resolves” guarantee is too strong. That misses at least armhf/i686 host layouts, leaving LD_PRELOAD unset and ALSA unavailable even though the bundled copy is removed. Add architecture-aware lookup or warn/fail when no host libasound.so.2 is found.

📍 Affects 5 files
  • data/apprun.sh#L75-L80 (this comment)
  • src/dependencywalker.h#L52-L54
  • src/miscdeploy.cpp#L173-L175
  • src/miscdeploy.h#L45-L46
  • README.md#L33-L33
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@data/apprun.sh` around lines 75 - 80, Update the AppRun preload loop at
data/apprun.sh:75-80 to discover libasound.so.2 using the host architecture’s
library paths, and warn or fail explicitly when none is found. Review the
related dependency handling at src/dependencywalker.h:52-54,
src/miscdeploy.cpp:173-175, and src/miscdeploy.h:45-46; make no direct changes
there unless needed to keep the dependency behavior consistent. Update the
“always resolves” claim in README.md:33 to reflect the architecture-aware lookup
and missing-library behavior.



# Set GStreamer paths
GST_PLUGIN_CORE_ELEMENTS="$(find "${ROOT}" -name "libgstcoreelements.so" -type f 2>/dev/null | head -1)"
Expand Down Expand Up @@ -109,6 +119,7 @@ echo "GDK_PIXBUF_MODULEDIR: ${GDK_PIXBUF_MODULEDIR}"
echo "GDK_PIXBUF_MODULE_FILE: ${GDK_PIXBUF_MODULE_FILE}"
echo "SSL_CERT_FILE: ${SSL_CERT_FILE}"
echo "GNUTLS_SYSTEM_PRIORITY_FILE: ${GNUTLS_SYSTEM_PRIORITY_FILE}"
echo "LD_PRELOAD: ${LD_PRELOAD}"
echo "GST_PLUGIN_PATH: ${GST_PLUGIN_PATH}"
echo "GST_PLUGIN_SYSTEM_PATH: ${GST_PLUGIN_SYSTEM_PATH}"
echo "GST_PLUGIN_SCANNER: ${GST_PLUGIN_SCANNER}"
Expand Down
2 changes: 1 addition & 1 deletion src/appimagedeploy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#include "gstreamerdeploy.h"
#include "qtdeploy.h"

// Top-level orchestration for the appimagedeploy tool: turns a prefix/ directory into a self-contained AppDir by discovering every ELF dependency, bundling desktop-integration support files (Gdk, Gtk, GStreamer, ALSA, PulseAudio, GLib schemas, fontconfig), Qt plugins/QML, the ELF interpreter, writing AppRun, copying and rpath-patching every dependency, and copying in copyright files.
// Top-level orchestration for the appimagedeploy tool: turns a prefix/ directory into a self-contained AppDir by discovering every ELF dependency, bundling desktop-integration support files (Gdk, Gtk, GStreamer, PulseAudio, GLib schemas, fontconfig), Qt plugins/QML, the ELF interpreter, writing AppRun, copying and rpath-patching every dependency, and copying in copyright files. ALSA is deliberately excluded rather than bundled (see MiscDeploy::HandleAlsa).
namespace AppImageDeploy {

// `desktop_file_path` is expected at <AppDir>/usr/share/applications/foo.desktop, as accepted by AppDir::Create(). `gstreamer_plugin_names` is only consulted when `gstreamer_plugin_set` is GStreamerDeploy::PluginSet::List, and `qt_sql_plugin_names` only when `qt_sql_plugin_set` is QtDeploy::SqlPluginSet::List. Returns false and sets *error_message on any unrecoverable failure.
Expand Down
8 changes: 8 additions & 0 deletions src/dependencywalker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,14 @@ QString DependencyWalker::FindLibrary(const QString &filename) {

}

void DependencyWalker::RemoveElfsByFilenamePrefix(const QString &filename_prefix) {

all_elfs_.removeIf([&filename_prefix](const QString &path) {
return QFileInfo(path).fileName().startsWith(filename_prefix);
});

}

QStringList DependencyWalker::FindWithPrefixInLibraryLocations(const QString &prefix) {

EnsureDefaultLibraryLocations();
Expand Down
3 changes: 3 additions & 0 deletions src/dependencywalker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ class DependencyWalker {
// With $QTDIR/$QT_ROOT_DIR added ahead of everything but rpath so a custom Qt build's own libraries are preferred over a same-named system copy. Returns an empty string if not found.
QString FindLibrary(const QString &filename);

// Removes every already-registered ELF whose filename starts with `filename_prefix` from all_elfs(), so it is never copied into the AppDir. library_locations() is left untouched - a stale search directory is harmless. For libraries that must always be resolved against the running system rather than bundled (e.g. ALSA): once removed here, the dynamic linker falls through the AppDir rpath (now empty for this name) to the host's own copy at runtime, same as for any other unbundled system library.
void RemoveElfsByFilenamePrefix(const QString &filename_prefix);

Comment on lines +52 to +54

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Document the preload requirement. RemoveElfsByFilenamePrefix() only removes the bundled copy; it doesn’t guarantee host resolution. Update the comment to say AppRun must explicitly preload a compatible host library instead of implying generic dynamic-linker fallback.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/dependencywalker.h` around lines 52 - 54, Update the documentation for
RemoveElfsByFilenamePrefix() to clarify that it only removes the bundled ELF and
does not guarantee host resolution. State that AppRun must explicitly preload a
compatible host library for libraries requiring system resolution, and remove
the implication that generic dynamic-linker fallback is sufficient.

private:
void EnsureDefaultLibraryLocations();
void AppendLib(const QString &path);
Expand Down
14 changes: 3 additions & 11 deletions src/miscdeploy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,17 +170,9 @@ void HandleAlsa(DependencyWalker &dependency_walker) {
for (const QString &lib : dependency_walker.all_elfs()) {
if (!QFileInfo(lib).fileName().startsWith("libasound.so"_L1)) continue;

qInfo() << "Bundling alsa-lib directory...";

const QStringList locations = dependency_walker.FindWithPrefixInLibraryLocations(u"alsa-lib"_s);
if (locations.isEmpty()) {
qWarning() << "Could not find alsa-lib directory";
qWarning() << "E.g., in Alpine Linux: apk add alsa-plugins alsa-plugins-pulse";
}
else {
qInfo() << "Bundling dependencies of alsa-lib directory...";
dependency_walker.AddElfTree(locations.first());
}
// ALSA's PCM/CTL name resolution (e.g. "default:CARD=...") depends on its plugin modules (dmix, dsnoop, pulse, pipewire, ...) and the /usr/share/alsa/{alsa.conf,cards,pcm,ucm2} snippets and hardware-specific UCM profiles maintained alongside it - all versioned together by the running distribution and whatever sound server is actually active on it. Bundling the build host's libasound.so and its plugin directory instead of the running system's reliably breaks device access (e.g. "Invalid argument" opening "default:CARD=...") even on a nominally identical host, once its alsa-lib/UCM data has moved on since the build. So ALSA must always come from the running system: remove it here rather than bundle it, the same way HandleNvidia refuses to bundle libnvidia*.
qInfo() << "Not bundling ALSA: removing libasound.so* so it resolves against the running system's alsa-lib at runtime";
dependency_walker.RemoveElfsByFilenamePrefix(u"libasound.so"_s);

break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/miscdeploy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ bool HandleGdk(DependencyWalker &dependency_walker, const AppDir &appdir, QStrin
// Returns false and sets error_message on failure.
bool DeployGtkDirectory(DependencyWalker &dependency_walker, const AppDir &appdir, const int gtk_version, QString &error_message);

// Bundles the alsa-lib plugin directory if libasound is a dependency. Best-effort: only warns if not found.
// Removes libasound.so* from the dependency set if present, so ALSA is always resolved against the running system's alsa-lib rather than bundled: its plugin modules, /usr/share/alsa config, and hardware UCM profiles are versioned together by the host and bundling them breaks device access on any host whose alsa-lib/UCM data doesn't exactly match the build host's (see miscdeploy.cpp for the failure mode this avoids).
void HandleAlsa(DependencyWalker &dependency_walker);
Comment on lines +45 to 46

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== files ==\n'
git ls-files | rg '(^|/)(src/miscdeploy\.h|src/miscdeploy\.cpp|README|readme|AppRun|app-run|desktop).*'

printf '\n== outline miscdeploy.h ==\n'
ast-grep outline src/miscdeploy.h --view expanded || true

printf '\n== outline miscdeploy.cpp ==\n'
ast-grep outline src/miscdeploy.cpp --view expanded || true

printf '\n== relevant text search ==\n'
rg -n --hidden -S 'HandleAlsa|libasound|ALSA|AppRun|preload|LD_LIBRARY_PATH|rpath|run.*system|bundled' src README* .github 2>/dev/null || true

Repository: jonaski/appimagetools

Length of output: 19964

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,140p' src/miscdeploy.h
printf '\n---\n'
sed -n '1,260p' src/miscdeploy.cpp

Repository: jonaski/appimagetools

Length of output: 15967

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the exact implementation around ALSA handling.
sed -n '1,220p' src/miscdeploy.h
printf '\n--- miscdeploy.cpp ---\n'
sed -n '1,260p' src/miscdeploy.cpp

printf '\n--- matches for AppRun / library resolution ---\n'
rg -n -S 'AppRun|LD_LIBRARY_PATH|libasound|ALSA|HandleAlsa|dependency_walker|bundled' . --glob '!**/.git/**' --glob '!**/node_modules/**' || true

Repository: jonaski/appimagetools

Length of output: 43840

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '60,90p' data/apprun.sh
printf '\n---\n'
sed -n '28,38p' README.md

Repository: jonaski/appimagetools

Length of output: 3833

Qualify the ALSA runtime contract. HandleAlsa() only removes libasound.so*; AppRun still has to find and LD_PRELOAD a host libasound.so.2 from a fixed path list. Document that host-layout dependency, or broaden the lookup if “always” is meant literally.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/miscdeploy.h` around lines 45 - 46, Update the ALSA runtime contract
around HandleAlsa so it explicitly documents that AppRun must locate and
LD_PRELOAD the host libasound.so.2 using the supported fixed path list, or
broaden that lookup to cover every intended host layout if the guarantee is
meant literally. Keep the dependency-walker behavior unchanged unless expanding
the lookup is required.


// Bundles the gio/modules directory (and its giomodule.cache, which unlike loaders.cache/immodules.cache holds no absolute paths so needs no patching) if libgio-2.0 is a dependency, so bundled GIO can still load its extra modules (gsettings backends, TLS backend, proxy resolvers, volume monitors) via GIO_EXTRA_MODULES. Best-effort: only warns if not found.
Expand Down