linuxdeployqt icon indicating copy to clipboard operation
linuxdeployqt copied to clipboard

Add all dependency rpaths to LD_LIBRARY_PATH for better Nix support

Open pedropombeiro opened this issue 6 years ago • 0 comments

I'm currently building a Qt app under Nix for deployment as an AppImage outside of Nix. I noticed that linuxdeployqt quickly stumbled when following library dependencies (for instance finding some libQt5DBus.so.5 but not libQt5Core.so.5). It seemed like it wasn't adding the rpaths to LD_LIBRARY_PATH while it was removing them from the dependencies. So I patched this area of the code:

https://github.com/probonopd/linuxdeployqt/blob/600fc20ea73ee937a402a2bb6b3663d93fcc1d4b/tools/linuxdeployqt/shared.cpp#L836

to look at all the components of the rpath and not just the first one. E.g if the rpath was $ORIGIN:/nix/store/... the current code would ignore it, but it should have added the /nix/store/... path to rpath. So I did the following:

diff --git a/tools/linuxdeployqt/shared.cpp b/tools/linuxdeployqt/shared.cpp
index 4c0919a..1a136e0 100644
--- a/tools/linuxdeployqt/shared.cpp
+++ b/tools/linuxdeployqt/shared.cpp
@@ -833,20 +833,23 @@ void changeIdentification(const QString &id, const QString &binaryPath)
     LogNormal() << "Checking rpath in" << binaryPath;
     QString oldRpath = runPatchelf(QStringList() << "--print-rpath" << binaryPath);
     LogDebug() << "oldRpath:" << oldRpath;
-    if (oldRpath.startsWith("/")){
-        LogDebug() << "Old rpath in" << binaryPath << "starts with /, hence adding it to LD_LIBRARY_PATH";
-        // FIXME: Split along ":" characters, check each one, only append to LD_LIBRARY_PATH if not already there
-        QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
-        QString oldPath = env.value("LD_LIBRARY_PATH");
-        if (not oldPath.contains(oldRpath)){
-            QString newPath = oldRpath + ":" + oldPath; // FIXME: If we use a ldd replacement, we still need to observe this path
-            // FIXME: Directory layout might be different for system Qt; cannot assume lib/ to always be inside the Qt directory
-            LogDebug() << "Added to LD_LIBRARY_PATH:" << newPath;
-            setenv("LD_LIBRARY_PATH",newPath.toUtf8().constData(),1);
+
+    QStringList rpath = oldRpath.split(":", QString::SkipEmptyParts);
+    foreach(QString path, QStringList(rpath)) {
+        if (path.startsWith("/")){
+            LogDebug() << "Old rpath in" << binaryPath << "starts with /, hence adding it to LD_LIBRARY_PATH";
+            // FIXME: Split along ":" characters, check each one, only append to LD_LIBRARY_PATH if not already there
+            QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
+            QString oldPath = env.value("LD_LIBRARY_PATH");
+            if (not oldPath.contains(oldRpath)){
+                QString newPath = oldRpath + ":" + oldPath; // FIXME: If we use a ldd replacement, we still need to observe this path
+                // FIXME: Directory layout might be different for system Qt; cannot assume lib/ to always be inside the Qt directory
+                LogDebug() << "Added to LD_LIBRARY_PATH:" << newPath;
+                setenv("LD_LIBRARY_PATH",newPath.toUtf8().constData(),1);
+            }
         }
     }
 
-    QStringList rpath = oldRpath.split(":", QString::SkipEmptyParts);
     rpath.prepend(id);
     rpath.removeDuplicates();
     foreach(QString path, QStringList(rpath)) {

and the app started working for me. Does it make sense to incorporate this change upstream?

pedropombeiro avatar Mar 11 '19 17:03 pedropombeiro