-
Notifications
You must be signed in to change notification settings - Fork 18
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
Update libfuse patch #37
Conversation
I've abandonned the idea of trying future fusermount versions for simplicity. |
Thanks for your contribution @Bqleine, really appreciate it. Never came across as rude, no need to apologize.
Imho that's actually a really important part of the solution. If you could add that back, I'd be all for it! |
Are you sure about this? I feel like the user would not expect that his AppImage searches for fusermount versions that don't exist, and it would confuse them if they're trying to debug problems. |
I would like to add that distros that only ship Imo, a more proper solution would be to automate the |
This is not what we want, as people would just leave their FUSE setup broken. |
So can we merge this? I don't want to implement the checking for future versions because I think its a bad idea for security and as @Samueru-sama pointed out it would not be very useful. Nevertheless, this fix is important as buffer overflows are important security concerns. |
I also want to point out that older versions of linux mint are affected by the current bug of the hardcoded fusermount path so hopefully this can be fixed soon. |
Yes, I'd also like to get a proper fix merged soon. To get this merged, we need to:
|
5dba66b
to
8551425
Compare
This new patch searches for fusermount and fusermount3..99 in both the compiled path, and in PATH. The error codes for each try are displayed in case of failure to find a suitable binary.
@probonopd What do you think of this new patch?
|
Thank you very much @Bqleine. Very helpful indeed. I think we should shorten the error output quite a bit - if no fusermount is found, we should probably just state that once. Is |
Build for testing: |
That’s just because we search for 99 versions, which seems a little bit over board.
We are really searching in that path, its the default path that fusermount was compiled with. The reason it’s « /var/empty » is simply because I built this image with GNU Guix. I could remove that second check though |
Couldn't we just say "No fusermount binary found on the $PATH" just once if none is found? |
It seems like FUSE nowadays also searches on the $PATH by default: They are using (I quickly checked: |
What if future versions of fusermount expect different parameters? I don't think it is a good idea to try unknown versions. Users can always add a symlink or wrapper script for fusermount to their system if they want to run ancient AppImages. |
Worst case is that it will break. If we do not even try, it will break for sure. So we better try at least. |
What's missing here to get this merged? |
We'd need @TheAssassin to review this. Friendly ping :) |
This new patch searches for fusermount and fusermount3 in both the compiled path, and in PATH. The error codes for each try are displayed in case of failure to find a suitable binary:
This pull request should close #36, #32, #16, #35, #31, and #15.
What do you think @probonopd @TheAssassin ?
P.S. sorry if I've been rude. I've made too much of a big deal out of things, I really appreciate your work on AppImages.