Do not traverse private deps

Previously, recursion was not terminated for private
dynamic libraries leading to slow performance.

Also, remove now redundant deduplication that happened
later.

Fixes: QBS-1714
Change-Id: Ia28f9993de8a55d6615c4ba0f7a4ef90721e31ff
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

6 files changed, 87 insertions, 5 deletions

diff --git a/share/qbs/modules/cpp/gcc.js b/share/qbs/modules/cpp/gcc.js
index 82f6889e4..90f8fc933 100644
--- a/share/qbs/modules/cpp/gcc.js
+++ b/share/qbs/modules/cpp/gcc.js
@@ -78,6 +78,7 @@ function useCompilerDriverLinker(product, inputs) {
 
 function collectLibraryDependencies(product, isDarwin) {
     var publicDeps = {};
+    var privateDeps = {};
     var objects = [];
     var objectByFilePath = {};
     var tagForLinkingAgainstSharedLib = product.cpp.imageFormat === "pe"
@@ -170,6 +171,8 @@ function collectLibraryDependencies(product, isDarwin) {
                 && typeof dep.artifacts[tagForLinkingAgainstSharedLib] !== "undefined";
         if (!isStaticLibrary && !isDynamicLibrary)
             return;
+        if (isBelowIndirectDynamicLib && privateDeps[dep.name])
+            return;
 
         var nextIsBelowIndirectDynamicLib = isBelowIndirectDynamicLib || isDynamicLibrary;
         dep.dependencies.forEach(function(depdep) {
@@ -193,6 +196,7 @@ function collectLibraryDependencies(product, isDarwin) {
                 publicDeps[dep.name] = true;
             } else {
                 addArtifactFilePaths(dep, tagForLinkingAgainstSharedLib, addPrivateFilePath);
+                privateDeps[dep.name] = true;
             }
         }
     }
@@ -204,7 +208,6 @@ function collectLibraryDependencies(product, isDarwin) {
     product.dependencies.forEach(traverseDirectDependency);
     addExternalLibs(product);
 
-    var seenRPathLinkDirs = {};
     var result = { libraries: [], rpath_link: [] };
     objects.forEach(
                 function (obj) {
@@ -215,10 +218,7 @@ function collectLibraryDependencies(product, isDarwin) {
                                                 framework: obj.framework });
                     } else {
                         var dirPath = FileInfo.path(obj.filePath);
-                        if (!seenRPathLinkDirs.hasOwnProperty(dirPath)) {
-                            seenRPathLinkDirs[dirPath] = true;
-                            result.rpath_link.push(dirPath);
-                        }
+                        result.rpath_link.push(dirPath);
                     }
                 });
     return result;
diff --git a/tests/auto/blackbox/testdata/rpathlink-deduplication/rpathlink-deduplication-lib.cpp b/tests/auto/blackbox/testdata/rpathlink-deduplication/rpathlink-deduplication-lib.cpp
new file mode 100644
index 000000000..6418df94d
--- /dev/null
+++ b/tests/auto/blackbox/testdata/rpathlink-deduplication/rpathlink-deduplication-lib.cpp
@@ -0,0 +1,3 @@
+int dynamicFunc() {
+    return 1;
+}
\ No newline at end of file
diff --git a/tests/auto/blackbox/testdata/rpathlink-deduplication/rpathlink-deduplication-main.cpp b/tests/auto/blackbox/testdata/rpathlink-deduplication/rpathlink-deduplication-main.cpp
new file mode 100644
index 000000000..60f8494f6
--- /dev/null
+++ b/tests/auto/blackbox/testdata/rpathlink-deduplication/rpathlink-deduplication-main.cpp
@@ -0,0 +1,5 @@
+extern int dynamicFunc();
+
+int main() {
+    return dynamicFunc();
+}
\ No newline at end of file
diff --git a/tests/auto/blackbox/testdata/rpathlink-deduplication/rpathlink-deduplication.qbs b/tests/auto/blackbox/testdata/rpathlink-deduplication/rpathlink-deduplication.qbs
new file mode 100644
index 000000000..adb63872a
--- /dev/null
+++ b/tests/auto/blackbox/testdata/rpathlink-deduplication/rpathlink-deduplication.qbs
@@ -0,0 +1,47 @@
+Project {
+    DynamicLibrary {
+        Depends { name: "bundle" }
+        Depends { name: "cpp" }
+
+        bundle.isBundle: false
+        name: "DynamicLibraryA"
+        files: ["rpathlink-deduplication-lib.cpp"]
+    }
+
+    DynamicLibrary {
+        Depends { name: "bundle" }
+        Depends { name: "cpp" }
+        Depends { name: "DynamicLibraryA" }
+
+        bundle.isBundle: false
+        name: "DynamicLibraryB"
+        files: ["rpathlink-deduplication-lib.cpp"]
+    }
+
+    DynamicLibrary {
+        Depends { name: "bundle" }
+        Depends { name: "cpp" }
+        Depends { name: "DynamicLibraryA" }
+
+        bundle.isBundle: false
+        name: "DynamicLibraryC"
+        files: ["rpathlink-deduplication-lib.cpp"]
+    }
+
+    CppApplication {
+        Depends { name: "bundle" }
+        Depends { name: "DynamicLibraryB" }
+        Depends { name: "DynamicLibraryC" }
+        consoleApplication: true
+        bundle.isBundle: false
+        cpp.removeDuplicateLibraries: false
+        files: "rpathlink-deduplication-main.cpp"
+        property bool test: {
+            if (cpp.useRPathLink)
+                console.info("useRPathLink: true");
+            else
+                console.info("useRPathLink: false");
+            console.info("===" + cpp.rpathLinkFlag + "===");
+        }
+    }
+}
\ No newline at end of file
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index cac8f6dc7..11a4078ea 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -2657,6 +2657,32 @@ void TestBlackbox::retaggedOutputArtifact()
     QVERIFY2(!QFile::exists(a3), qPrintable(a3));
 }
 
+void TestBlackbox::rpathlinkDeduplication()
+{
+    QDir::setCurrent(testDataDir + "/rpathlink-deduplication");
+    QbsRunParameters resolveParams{"resolve"};
+    QCOMPARE(runQbs(resolveParams), 0);
+    const bool useRPathLink = m_qbsStdout.contains("useRPathLink: true");
+    const bool dontUseRPathLink = m_qbsStdout.contains("useRPathLink: false");
+    QVERIFY2(useRPathLink || dontUseRPathLink, m_qbsStdout);
+    if (dontUseRPathLink)
+        QSKIP("Only applies to toolchains that support rPathLink");
+    const QString output = QString::fromLocal8Bit(m_qbsStdout);
+    const QRegularExpression pattern(QRegularExpression::anchoredPattern(".*===(.*)===.*"),
+                                     QRegularExpression::DotMatchesEverythingOption);
+    const QRegularExpressionMatch match = pattern.match(output);
+    QVERIFY2(match.hasMatch(), qPrintable(output));
+    QCOMPARE(pattern.captureCount(), 1);
+    const QString linkFlag = match.captured(1);
+
+    QbsRunParameters buildParams;
+    buildParams.arguments = QStringList({"--command-echo-mode", "command-line"});
+    QCOMPARE(runQbs(buildParams), 0);
+    // private DynamicLibraryA is a dependency for 2 other libs but should only appear once
+    const auto libDir = QFileInfo(relativeProductBuildDir("DynamicLibraryA")).absoluteFilePath();
+    QCOMPARE(m_qbsStdout.count((linkFlag + libDir).toUtf8()), 1);
+}
+
 void TestBlackbox::ruleConditions()
 {
     QDir::setCurrent(testDataDir + "/ruleConditions");
diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h
index b1ec6877f..f878fd89e 100644
--- a/tests/auto/blackbox/tst_blackbox.h
+++ b/tests/auto/blackbox/tst_blackbox.h
@@ -283,6 +283,7 @@ private slots:
     void rescueTransformerData();
     void responseFiles();
     void retaggedOutputArtifact();
+    void rpathlinkDeduplication();
     void ruleConditions();
     void ruleConnectionWithExcludedInputs();
     void ruleCycle();