AppImage: Un-pollute /usr/lib

[tresf]

Remove some workaround code that was added as part of #7252 due to a misunderstanding of how linuxdeploy works.

This workaround is best explained here:

lmms/cmake/linux/apprun-hooks/usr-lib-hooks.sh

Lines 3 to 8 in fbac56f

	# Workaround libraries being incorrectly placed in usr/lib (e.g. instead of usr/lib/lmms, etc)
	# FIXME: Remove when linuxdeploy supports subfolders https://github.com/linuxdeploy/linuxdeploy/issues/305
	DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." && pwd )"
	export LMMS_PLUGIN_DIR="$DIR/usr/lib/"
	export LADSPA_PATH="$DIR/usr/lib/"
	export SUIL_MODULE_DIR="$DIR/usr/lib/"

Related upstream (wontfix) bug report:

• Subfolder handling in /usr/lib linuxdeploy/linuxdeploy#305

Linux testing TODO:

• Test ARM64 AppImage
  • Test LADSPA plugins
  • Test ZynAddSubFx
  • Test STK/Mallets
  • Test Samples/Waveforms
• Test Intel Appimage
  • Test LADSPA plugins
  • Test Wine 32-bit VSTs
  • Test Wine 64-bit VSTs
  • Test Linux Native VSTs
  • Test ZynAddSubFx
  • Test STK/Mallets
  • Test Samples/Waveforms

I'm not sure the state of Suil plugins, so they may be testable too? Pinging @JohannesLorenz for any info on the status of that on master branch.

[tresf]

Surprisingly, all tests pass. Would still like a second set of eyes, but no regressions found with the plugin system.

[TheAssassin]

The amount of lines removed vs. lines added is really great to see.

[messmerd]

I plan to test this, but it might not be until Sunday when I have the time

[tresf]

I half expected this but linuxdeploy doesn't like non-existent directories as parameters to --deploy-deps-only; this PR requires some slight refactoring to know what we're bundling.

[JohannesLorenz]

I'm not sure the state of Suil plugins, so they may be testable too?

Suil was added back then with #4899 , just as a preparation for #7201 . Testing Suil properly would mean rebasing a copy of #7201 on this PR. I could try this on a weekend, though my time is very limited in September. If you need a fast test, someone else could try it out, too. Anyways, let me know when you think this PR is ready for testing.

[tresf]

I'm not sure the state of Suil plugins, so they may be testable too?

Suil was added back then with #4899 , just as a preparation for #7201 . Testing Suil properly would mean rebasing a copy of #7201 on this PR. I could try this on a weekend, though my time is very limited in September. If you need a fast test, someone else could try it out, too. Anyways, let me know when you think this PR is ready for testing.

I had a feeling it wasn't merged yet. No worries then @JohannesLorenz we can tackle that when #7201 is closer to being merged. From what I recall, the path to suil-0 needs to be specified in an environment variable, which I've modified as part of this PR, but shouldn't cause any impact to nightly since it's currently unused.

[tresf]

Mostly unrelated to this PR, but I'm adding it anyways... after revisiting copy_excluded(...) macro that's was part of LinuxDeploy.cmake, I've decided to give it a better name CopyDependency.cmake/copy_dependency(...) and move it to a dedicated cmake module via 2d93cf8.

Details below for those that are interested in the TL;DR.

Click to expand details

---

Originally, the intent of this macro was to make ldd foo.so |grep some_dep.so.0 a bit easier from cmake so that we could isolate (e.g. move) "excluded" libraries, such as libjack.so.0 into a safe location for systems that ship with it, but also provide it dynamically at startup for systems without (e.g. #7689) that would otherwise break the .AppImage from loading. ldd is used to get the EXACT name and location of the dependency and then we resolve any symlinks to ensure we're copying the physical file.

lmms/cmake/linux/apprun-hooks/jack-hook.sh

Lines 8 to 13 in fbac56f

	if ldd "$DIR/usr/bin/lmms" |grep "libjack.so" |grep "not found" > /dev/null 2>&1; then
	echo "[$ME] Jack does not appear to be installed. That's OK, we'll use a dummy version instead." >&2
	export LD_LIBRARY_PATH="$DIR/usr/lib/jack:$LD_LIBRARY_PATH"
	else
	echo "[$ME] Jack appears to be installed on this system, so we'll use it." >&2
	fi

Where we ran into trouble (hence #7689) was the missing transient dependency, which required us to recursively call this function again against libjack.so.0 to resolve libdb-5.3.so, which we create a symlinks for... But this won't scale if other dependencies are needed...

However, with the advent of --deploy-deps-only, we can do this proactively by stashing libjack.so.0 into a dedicated directory and asking linuxdeploy to handle our dependency checks (e.g. libdb-5.3.so) while also fixing the rpath entries for this one-off lib.

In addition, I've also written the macro in a way that it should be reusable on other OSs for the same purpose if we need it into the future:

• Tested on macOS (uses otool -L instead of ldd)
• Tested on FreeBSD (file and ldd behave identically)
• Tested on OpenBSD (file and ldd behave identically)
• Tested on Haiku (for posterity) which uses application/x-sharedlib but there's no ldd equivalent, ignoring.
• Tested on Solaris 11, but it doesn't support file -b --mime-type, ignoring.

[bratpeki]

Debloating is always great. How do I test this?

[tresf]

Debloating is always great. How do I test this?

Most of the testing is bulleted here: #8053 (comment)

@messmerd usually does a file count/tree comparison (e.g. BEFORE vs. AFTER) which is nice too as a sanity check. I would expect everything to work, just need a second set of eyes.

[messmerd]

@tresf
I compared the squashfs-root/ contents of this PR's AppImages to nightly's and everything looks fine.

I also tested it briefly and didn't notice any issues.

In #7252, you added what I believe is a workaround with the LMMS_PLUGIN_DIR environment variable to src/core/RemotePlugin.cpp - maybe that can be removed now? And I seem to remember some other workarounds too. Maybe in #7722 or #7691.

[tresf]

In #7252, you added what I believe is a workaround with the LMMS_PLUGIN_DIR environment variable to src/core/RemotePlugin.cpp - maybe that can be removed now?

Good catch, agreed.

And I seem to remember some other workarounds too. Maybe in #7722 or #7691.

Hmm... I don't see any compelling reason to revert any of this code. I don't see any disadvantage in fine-grained control at runtime for our plugin scanning. Notice that there are no TODO or FIXME entries in any of that code because it provides a useful feature for problematic or missing plugins.... meanwhile I can see several cases into the future where a user may need to leverage these.

[tresf]

In #7252, you added what I believe is a workaround with the LMMS_PLUGIN_DIR environment variable to src/core/RemotePlugin.cpp

Clarification:

• LMMS_EXCLUDE_LADSPA: Yes.
• LMMS_EXCLUDE_PLUGINS: No.

Reasoning: Our own libcarla plugin still needs to be excluded to silence unnecessary warnings from startup; This is a quality-of-life improvement unrelated to the workaround code.

• Allow plugins to be skipped at runtime #7691

[messmerd]

@tresf That sounds reasonable to me.

If you're removing LMMS_EXCLUDE_LADSPA, remember there are also the C++-side changes in src/core/LadspaManager.cpp.

[tresf]

@tresf That sounds reasonable to me.

If you're removing LMMS_EXCLUDE_LADSPA, remember there are also the C++-side changes in src/core/LadspaManager.cpp.

Quoting my previous stance on this:

Hmm... I don't see any compelling reason to revert any of this code. I don't see any disadvantage in fine-grained control at runtime for our plugin scanning. Notice that there are no TODO or FIXME entries in any of that code because it provides a useful feature for problematic or missing plugins.... meanwhile I can see several cases into the future where a user may need to leverage these.

[messmerd]

LGTM