D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R241:ce148291392c: rename internal kioslave helper executable to kioslave5 (authored by cullmann). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8810?vs=63931=64556 REVISION DETAIL https://phabricator.kde.org/D8810 AFFECTED FILES src/core/slave.cpp src/kioslave/CMakeLists.txt src/kioslave/kioslave.cpp To: cullmann, #frameworks, kfunk, kkofler, dfaure Cc: kde-frameworks-devel, dfaure, ngraham, broulik, kfunk, LeGast00n, GB_2, michaelh, bruns
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
kkofler added a comment. If you ask me (there are 2 Kevins on this bug), I think this will work. I think it's kinda overkill (it pretty much defeats the point of having version-specific libexecdirs if we end up renaming the binaries anyway), but if it works (which should be the case), I'm OK with it. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8810 To: cullmann, #frameworks, kfunk, kkofler, dfaure Cc: kde-frameworks-devel, dfaure, ngraham, broulik, kfunk, LeGast00n, GB_2, michaelh, bruns
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
cullmann added a comment. Ok, Kevin, ok with this variant, too? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8810 To: cullmann, #frameworks, kfunk, kkofler, dfaure Cc: kde-frameworks-devel, dfaure, ngraham, broulik, kfunk, LeGast00n, GB_2, michaelh, bruns
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
dfaure accepted this revision. dfaure added a comment. Seems to work for me. But it's weird that the KIO unittests don't work for you... we should fix that. Make sure to edit the commit log so it doesn't look like this RR's description anymore, if needed. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8810 To: cullmann, #frameworks, kfunk, kkofler, dfaure Cc: kde-frameworks-devel, dfaure, ngraham, broulik, kfunk, LeGast00n, GB_2, michaelh, bruns
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
cullmann added a reviewer: dfaure. cullmann added a comment. Could somebody try this? Unfortunately, neither with or without this patch I can get all kio unit tests running here properly :( REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8810 To: cullmann, #frameworks, kfunk, kkofler, dfaure Cc: kde-frameworks-devel, dfaure, ngraham, broulik, kfunk, LeGast00n, michaelh, bruns
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
cullmann updated this revision to Diff 63931. cullmann added a comment. Instead of changing the search paths, alter the kioslave binary name to "kioslave5" to avoid clashs with old variants. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8810?vs=22338=63931 REVISION DETAIL https://phabricator.kde.org/D8810 AFFECTED FILES src/core/slave.cpp src/kioslave/CMakeLists.txt src/kioslave/kioslave.cpp To: cullmann, #frameworks, kfunk, kkofler Cc: kde-frameworks-devel, dfaure, ngraham, broulik, kfunk, LeGast00n, michaelh, bruns
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
cullmann commandeered this revision. cullmann edited reviewers, added: kkofler; removed: cullmann. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8810 To: cullmann, #frameworks, kfunk, kkofler Cc: kde-frameworks-devel, dfaure, ngraham, broulik, kfunk, LeGast00n, michaelh, bruns
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
kkofler added a comment. Herald added a subscriber: kde-frameworks-devel. Ping? This has been stuck for over a year now. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8810 To: kkofler, #frameworks, kfunk, cullmann Cc: kde-frameworks-devel, dfaure, ngraham, broulik, kfunk, michaelh, bruns
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
dfaure added a comment. Isn't it time to uninstall kdelibs3? :-) Otherwise yes I'm ok with you renaming the helper executable, its use is very self-contained. Only KIO can call it, it takes a path to a socket created by KIO... REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8810 To: kkofler, #frameworks, kfunk, cullmann Cc: dfaure, ngraham, broulik, kfunk
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
cullmann added a comment. I can live with that patch, thought I still think we should just rename the helper, but perhaps that breaks other things if people relied on the name. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8810 To: kkofler, #frameworks, kfunk, cullmann Cc: ngraham, broulik, kfunk
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
kkofler added a comment. And the historical reason the name was not suffixed is because the binary was moved to a private libexec subdirectory to prevent conflicts. This worked fine as long as `/usr/bin` did not end up in the search path (in particular, for the whole 4.x series). REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8810 To: kkofler, #frameworks, kfunk, cullmann Cc: broulik, kfunk
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
kkofler added a comment. In https://phabricator.kde.org/D8810#167568, @cullmann wrote: > Yeah, that was done to be able to provide win/linux install bundles. A GNU/Linux install bundle should have a reparented FHS-like tree, possibly wrapped in an image file (Flatpak/Snap/AppImage). In such a tree, the libexec binaries will never be in `$prefix/bin` (the applicationDirPath), but always somewhere under `$prefix/libexec` or `$prefix/lib`. Windows and macOS are different and I believe you when you say that looking in the applicationDirPath makes sense there. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8810 To: kkofler, #frameworks, kfunk, cullmann Cc: broulik, kfunk
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
cullmann added a comment. I can live with that ;=) But still I think kioslave5 as name would make this not needed and be more in line with kded5/... REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8810 To: kkofler, #frameworks, kfunk, cullmann Cc: broulik, kfunk
D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)
kkofler updated this revision to Diff 22338. kkofler retitled this revision from "Do not look for kioslave binary in applicationDirPath (#386859)" to "Do not look for kioslave binary in applicationDirPath on *nix (#386859)". kkofler edited the summary of this revision. kkofler added a comment. So, since this line makes sense on Windows/Mac, let's only get rid of it on Q_OS_UNIX then. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8810?vs=22305=22338 REVISION DETAIL https://phabricator.kde.org/D8810 AFFECTED FILES src/core/slave.cpp To: kkofler, #frameworks, kfunk, cullmann Cc: broulik, kfunk
