-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
AppImage: Un-pollute /usr/lib #8053
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.
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
base: master
Are you sure you want to change the base?
Conversation
Surprisingly, all tests pass. Would still like a second set of eyes, but no regressions found with the plugin system. |
The amount of lines removed vs. lines added is really great to see. |
I plan to test this, but it might not be until Sunday when I have the time |
I half expected this but |
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 |
Mostly unrelated to this PR, but I'm adding it anyways... after revisiting Details below for those that are interested in the TL;DR. Click to expand detailsOriginally, the intent of this macro was to make lmms/cmake/linux/apprun-hooks/jack-hook.sh Lines 8 to 13 in fbac56f
Where we ran into trouble (hence #7689) was the missing transient dependency, which required us to recursively call this function again against However, with the advent of 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:
|
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. |
@tresf I also tested it briefly and didn't notice any issues. In #7252, you added what I believe is a workaround with the |
Good catch, agreed.
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 |
…arent folder with LADSPA plugins) This reverts commit 8b160c4.
8b160c4
to
2ffabfe
Compare
Clarification:
Reasoning: Our own |
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
Related upstream (wontfix) bug report:
Linux testing TODO:
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.