AppRun.c/$LD_LIBRARY_PATH break AppImageLauncher processes

[capezotte]

Describe the bug
Recipes/scripts from @probonopd's pkg2appimage which use the built-in convert binary (which is itself an AppImage) crash before using this command, with the error message:

/usr/bin/AppImageLauncher: /tmp/.mount_pkg2apnDXHnv/usr/lib/x86_64-linux-gnu/liblzma.so.5: version 'XZ_5.2' not found (required by /usr/lib/libarchive.so.13)
i.e. AppImageLauncher tries to integrate the convert AppImage within pkg2appimage, but due to the latter setting up environment variables such as LD_LIBRARY_PATH, conflicts occur.

Expected behavior
AppImageLancher should be able to detect AppImages running within AppImages.

To reproduce
File: example.yml

app: convert-test

ingredients:
  script:
    - wget "https://www.iana.org/_img/2013.1/iana-logo-header.svg" -O iana.svg
    - convert iana.svg iana.png

Run with through pkg2appimage: ./pkg2appimage-1794-x86_64.AppImage example.yml

Right after the output line:
+ convert iana.svg iana.png

The error message /usr/bin/AppImageLauncher: /tmp/.mount_pkg2apnDXHnv/usr/lib/x86_64-linux-gnu/liblzma.so.5: version 'XZ_5.2' not found (required by /usr/lib/libarchive.so.13) will appear and the program will halt.

System, software and AppImage information
OS: Arch Linux (last updated: 2020-10-20)
AppImageLauncher version: 2.2.0 (AUR Release: 1)
pkg2appimage: continuous, 1794

[TheAssassin]

Note that AppImageLauncher doesn't necessarily try to integrate that binary. It is just the entry point for every execution of an AppImage, it will always run. If it should integrate or not is decided during runtime. For those tools, I could well imagine it would just move on to running the actual AppImage, if it didn't crash in the loader.

I can only guess that your AppImage's AppRun sets $LD_LIBRARY_PATH, which affects AppImageLauncher. There's no easy way to prevent AppImageLauncher from failing in this scenario. Perhaps the only way could be an intermediate, statically built binary that first resets $LD_LIBRARY_PATH before running AppImageLauncher, but that'll have no way to understand how it's been altered by your AppImage and would have to erase it entirely. Building AppImageLauncher itself statically is not a realistic option due to the licensing of Qt (and also, distros don't like this).

This is why I really hate AppRun.c in AppImageKit, and have wanted to remove it for years. linuxdeploy and all the other tools using the DT_RUNPATH instead of $LD_LIBRARY_PATH would never cause such issues.

@probonopd can you please build your pkg2appimage thing without AppRun.c?

[probonopd]

@TheAssassin if I am not mistaken, the error occurs in https://github.com/AppImage/pkg2appimage/releases/tag/continuous, not in any of @OC1024's own AppImages. The particularity about this AppImage is that it contains another AppImage inside it. (While not generally considered best practice, this can be useful in some situations.)

AppRun sets $LD_LIBRARY_PATH, which affects AppImageLauncher

Can you elaborate please? I think many (if not most) AppImages do this. So even if we change this one AppImage, the problem will likely re-occur in other AppImages?

[TheAssassin]

I think many (if not most) AppImages do this.

There is a reason we discourage people from using environment variables, and try to use whatever else is possible. You brought up rpath yourself in linuxdeployqt, actually. Using $LD_LIBRARY_PATH or any other environment variable should really be an anti pattern, as it influences any third party process.

The obvious solution is, if you use $LD_LIBRARY_PATH, you need to reset your environment and remove the offending value before calling subprocesses. That's been a best practice ever since in programming, it's not really a new concept.

For instance, AppImageLauncher's binfmt-bypass uses $LD_PRELOAD to "hack" the AppImage runtime. The injected methods ensure $LD_PRELOAD is unset again, though, as soon as either of the functions are called. This ensures applications within the AppImage do not $LD_PRELOAD the same library, which would likely break them.

As $LD_LIBRARY_PATH outrules e.g., DT_RUNPATH and I don't control the actual launch of AppImageLauncher, the only chance I see to work around in AppImageLauncher is to have a little, static binary in place which resets the environment variable, and then calls AppImageLauncher's main binary. But that's cumbersome, and won't solve the issue in other situations.

This is, yet again, a documentary issue. Developers must be educated that these environment variables are harmful.

Edit: regarding pkg2appimage, the antipattern is implemented by AppRun.c, which I think it still uses. The quickest way to work around the issue is to have your shell script basically unset LD_LIBRARY_PATH. I don't think anyone really needs $LD_LIBRARY_PATH to influence how AppImages are run...

I might still implement that tiny wrapper binary. It could back up the values of $LD_LIBRARY_PATH and $LD_PRELOAD, then have AppImageLauncher reinsert it before calling binfmt-bypass. That'll ensure that AppImageLauncher has no observable influence on the execution of applications.

But the real issue is that this antipattern is still being relied on. And that would be fine. But you are then also responsible for cleaning up the environment before running other tools.