From a8bb88572d2f0c39d3841d082cd956593cfe403e Mon Sep 17 00:00:00 2001 From: Alberto Mardegan Date: Thu, 7 Nov 2019 20:06:41 +0300 Subject: GCC: avoid duplicating objects in linker command line Long command lines are especially a problem when building statically in MXE. This commit tries to replicate the logic employed by qmake, where before adding a new object to the command line, all of its previous appearences get removed. Fixes: QBS-1273 Change-Id: I51c843c4a7cfc170ed4fab30deae6c4229690dd0 Reviewed-by: Christian Kandeler --- doc/reference/modules/cpp-module.qdoc | 10 ++ share/qbs/modules/cpp/CppModule.qbs | 1 + share/qbs/modules/cpp/gcc.js | 12 +++ .../testdata/linker-library-duplicates/lib1.cpp | 3 + .../testdata/linker-library-duplicates/lib2.cpp | 3 + .../testdata/linker-library-duplicates/lib3.cpp | 3 + .../testdata/linker-library-duplicates/main.cpp | 13 +++ .../setup-run-environment.qbs | 48 ++++++++++ .../testdata/remove-duplicate-libs/MyStaticLib.qbs | 10 ++ .../blackbox/testdata/remove-duplicate-libs/main.c | 9 ++ .../testdata/remove-duplicate-libs/provider.c | 1 + .../testdata/remove-duplicate-libs/provider2.c | 1 + .../remove-duplicate-libs.qbs | 26 ++++++ .../testdata/remove-duplicate-libs/requestor1.c | 3 + .../testdata/remove-duplicate-libs/requestor2.c | 3 + tests/auto/blackbox/tst_blackbox.cpp | 102 +++++++++++++++++++++ tests/auto/blackbox/tst_blackbox.h | 4 + 17 files changed, 252 insertions(+) create mode 100644 tests/auto/blackbox/testdata/linker-library-duplicates/lib1.cpp create mode 100644 tests/auto/blackbox/testdata/linker-library-duplicates/lib2.cpp create mode 100644 tests/auto/blackbox/testdata/linker-library-duplicates/lib3.cpp create mode 100644 tests/auto/blackbox/testdata/linker-library-duplicates/main.cpp create mode 100644 tests/auto/blackbox/testdata/linker-library-duplicates/setup-run-environment.qbs create mode 100644 tests/auto/blackbox/testdata/remove-duplicate-libs/MyStaticLib.qbs create mode 100644 tests/auto/blackbox/testdata/remove-duplicate-libs/main.c create mode 100644 tests/auto/blackbox/testdata/remove-duplicate-libs/provider.c create mode 100644 tests/auto/blackbox/testdata/remove-duplicate-libs/provider2.c create mode 100644 tests/auto/blackbox/testdata/remove-duplicate-libs/remove-duplicate-libs.qbs create mode 100644 tests/auto/blackbox/testdata/remove-duplicate-libs/requestor1.c create mode 100644 tests/auto/blackbox/testdata/remove-duplicate-libs/requestor2.c diff --git a/doc/reference/modules/cpp-module.qdoc b/doc/reference/modules/cpp-module.qdoc index 2f7f06698..64a7c071f 100644 --- a/doc/reference/modules/cpp-module.qdoc +++ b/doc/reference/modules/cpp-module.qdoc @@ -1699,3 +1699,13 @@ \defaultvalue \c{false} */ + +/*! + \qmlproperty bool cpp::removeDuplicateLibraries + \since Qbs 1.16 + + Whether the list of the objects and libraries is reduced to a + list of unique values before being passed to the linker. + + \defaultvalue \c{true} +*/ diff --git a/share/qbs/modules/cpp/CppModule.qbs b/share/qbs/modules/cpp/CppModule.qbs index efda896da..bdd6d2750 100644 --- a/share/qbs/modules/cpp/CppModule.qbs +++ b/share/qbs/modules/cpp/CppModule.qbs @@ -191,6 +191,7 @@ Module { property bool useRPathLink property string rpathLinkFlag property bool discardUnusedData + property bool removeDuplicateLibraries: true property stringList assemblerFlags PropertyOptions { diff --git a/share/qbs/modules/cpp/gcc.js b/share/qbs/modules/cpp/gcc.js index 639191da6..574fe769b 100644 --- a/share/qbs/modules/cpp/gcc.js +++ b/share/qbs/modules/cpp/gcc.js @@ -80,8 +80,20 @@ function collectLibraryDependencies(product, isDarwin) { var objectByFilePath = {}; var tagForLinkingAgainstSharedLib = product.cpp.imageFormat === "pe" ? "dynamiclibrary_import" : "dynamiclibrary"; + var removeDuplicateLibraries = product.cpp.removeDuplicateLibraries function addObject(obj, addFunc) { + /* If the object is already known, remove its previous usage and insert + * it again in the new desired position. This preserves the order of + * the other objects, and is analogous to what qmake does (see the + * mergeLflags parameter in UnixMakefileGenerator::findLibraries()). + */ + if (removeDuplicateLibraries && (obj.filePath in objectByFilePath)) { + var oldObj = objectByFilePath[obj.filePath]; + var i = objects.indexOf(oldObj); + if (i >= 0) + objects.splice(i, 1); + } addFunc.call(objects, obj); objectByFilePath[obj.filePath] = obj; } diff --git a/tests/auto/blackbox/testdata/linker-library-duplicates/lib1.cpp b/tests/auto/blackbox/testdata/linker-library-duplicates/lib1.cpp new file mode 100644 index 000000000..f2db0a751 --- /dev/null +++ b/tests/auto/blackbox/testdata/linker-library-duplicates/lib1.cpp @@ -0,0 +1,3 @@ +#include "../dllexport.h" + +DLL_EXPORT void lib1Func() { } diff --git a/tests/auto/blackbox/testdata/linker-library-duplicates/lib2.cpp b/tests/auto/blackbox/testdata/linker-library-duplicates/lib2.cpp new file mode 100644 index 000000000..4042b2d35 --- /dev/null +++ b/tests/auto/blackbox/testdata/linker-library-duplicates/lib2.cpp @@ -0,0 +1,3 @@ +#include "../dllexport.h" + +DLL_EXPORT void lib2Func() { } diff --git a/tests/auto/blackbox/testdata/linker-library-duplicates/lib3.cpp b/tests/auto/blackbox/testdata/linker-library-duplicates/lib3.cpp new file mode 100644 index 000000000..0f0183f69 --- /dev/null +++ b/tests/auto/blackbox/testdata/linker-library-duplicates/lib3.cpp @@ -0,0 +1,3 @@ +#include "../dllexport.h" + +DLL_EXPORT void lib3Func() { } diff --git a/tests/auto/blackbox/testdata/linker-library-duplicates/main.cpp b/tests/auto/blackbox/testdata/linker-library-duplicates/main.cpp new file mode 100644 index 000000000..ab698e90b --- /dev/null +++ b/tests/auto/blackbox/testdata/linker-library-duplicates/main.cpp @@ -0,0 +1,13 @@ +#include "../dllexport.h" + +DLL_IMPORT void lib1Func(); +DLL_IMPORT void lib2Func(); +DLL_IMPORT void lib3Func(); + +int main() +{ + lib1Func(); + lib2Func(); + lib3Func(); + return 0; +} diff --git a/tests/auto/blackbox/testdata/linker-library-duplicates/setup-run-environment.qbs b/tests/auto/blackbox/testdata/linker-library-duplicates/setup-run-environment.qbs new file mode 100644 index 000000000..9723fd3f5 --- /dev/null +++ b/tests/auto/blackbox/testdata/linker-library-duplicates/setup-run-environment.qbs @@ -0,0 +1,48 @@ +import qbs 1.0 + +Project { + DynamicLibrary { + id: idLib1 + name: "lib1" + Depends { name: "cpp" } + files: ["lib1.cpp"] + Depends { name: "bundle" } + bundle.isBundle: false + } + + DynamicLibrary { + id: idLib2 + name: "lib2" + Depends { name: "cpp" } + files: ["lib2.cpp"] + Depends { name: "bundle" } + bundle.isBundle: false + } + + DynamicLibrary { + id: idLib3 + name: "lib3" + Depends { name: "cpp" } + files: ["lib3.cpp"] + Depends { name: "bundle" } + bundle.isBundle: false + } + + CppApplication { + name: "main" + files: "main.cpp" + + Depends { name: "lib1"; cpp.link: false } + Depends { name: "lib2"; cpp.link: false } + Depends { name: "lib3"; cpp.link: false } + + cpp.libraryPaths: [ + idLib1.buildDirectory, + idLib2.buildDirectory, + idLib3.buildDirectory + ] + cpp.dynamicLibraries: [ + "lib1", "lib2", "lib1", "lib3", "lib2", "lib1" + ] + } +} diff --git a/tests/auto/blackbox/testdata/remove-duplicate-libs/MyStaticLib.qbs b/tests/auto/blackbox/testdata/remove-duplicate-libs/MyStaticLib.qbs new file mode 100644 index 000000000..bd79759eb --- /dev/null +++ b/tests/auto/blackbox/testdata/remove-duplicate-libs/MyStaticLib.qbs @@ -0,0 +1,10 @@ +StaticLibrary { + Properties { + condition: isForDarwin + bundle.isBundle: false + } + + Depends { name: "cpp" } + files: name + ".c" + destinationDirectory: project.libDir +} diff --git a/tests/auto/blackbox/testdata/remove-duplicate-libs/main.c b/tests/auto/blackbox/testdata/remove-duplicate-libs/main.c new file mode 100644 index 000000000..5b23b6f58 --- /dev/null +++ b/tests/auto/blackbox/testdata/remove-duplicate-libs/main.c @@ -0,0 +1,9 @@ +void requestor1(); +void requestor2(); + +int main(void) +{ + requestor1(); + requestor2(); + return 0; +} diff --git a/tests/auto/blackbox/testdata/remove-duplicate-libs/provider.c b/tests/auto/blackbox/testdata/remove-duplicate-libs/provider.c new file mode 100644 index 000000000..25a85319a --- /dev/null +++ b/tests/auto/blackbox/testdata/remove-duplicate-libs/provider.c @@ -0,0 +1 @@ +void provider1() {} diff --git a/tests/auto/blackbox/testdata/remove-duplicate-libs/provider2.c b/tests/auto/blackbox/testdata/remove-duplicate-libs/provider2.c new file mode 100644 index 000000000..d7cdce832 --- /dev/null +++ b/tests/auto/blackbox/testdata/remove-duplicate-libs/provider2.c @@ -0,0 +1 @@ +void provider2() {} diff --git a/tests/auto/blackbox/testdata/remove-duplicate-libs/remove-duplicate-libs.qbs b/tests/auto/blackbox/testdata/remove-duplicate-libs/remove-duplicate-libs.qbs new file mode 100644 index 000000000..4ffb8d0e2 --- /dev/null +++ b/tests/auto/blackbox/testdata/remove-duplicate-libs/remove-duplicate-libs.qbs @@ -0,0 +1,26 @@ +import "MyStaticLib.qbs" as MyStaticLib + +Project { + property bool removeDuplicates + property string libDir: buildDirectory + "/lib" + property bool dummy: { + console.info("is bfd linker: " + + (qbs.toolchain.contains("gcc") && !qbs.hostOS.contains("macos"))) + } + + qbsSearchPaths: "." + MyStaticLib { name: "requestor1" } + MyStaticLib { name: "requestor2" } + MyStaticLib { name: "provider"; Group { files: "provider2.c" } } + + CppApplication { + consoleApplication: true + Depends { name: "requestor1"; cpp.link: false } + Depends { name: "requestor2"; cpp.link: false } + Depends { name: "provider"; cpp.link: false } + cpp.libraryPaths: project.libDir + cpp.removeDuplicateLibraries: project.removeDuplicates + cpp.staticLibraries: ["requestor1", "requestor2", "provider", "requestor2"] + files: "main.c" + } +} diff --git a/tests/auto/blackbox/testdata/remove-duplicate-libs/requestor1.c b/tests/auto/blackbox/testdata/remove-duplicate-libs/requestor1.c new file mode 100644 index 000000000..6d6b1a2da --- /dev/null +++ b/tests/auto/blackbox/testdata/remove-duplicate-libs/requestor1.c @@ -0,0 +1,3 @@ +void provider1(); + +void requestor1() { provider1(); } diff --git a/tests/auto/blackbox/testdata/remove-duplicate-libs/requestor2.c b/tests/auto/blackbox/testdata/remove-duplicate-libs/requestor2.c new file mode 100644 index 000000000..fa50d1d63 --- /dev/null +++ b/tests/auto/blackbox/testdata/remove-duplicate-libs/requestor2.c @@ -0,0 +1,3 @@ +void provider2(); + +void requestor2() { provider2(); } diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp index 72f610364..30f2482be 100644 --- a/tests/auto/blackbox/tst_blackbox.cpp +++ b/tests/auto/blackbox/tst_blackbox.cpp @@ -2264,6 +2264,28 @@ void TestBlackbox::referenceErrorInExport() "referenceErrorInExport.qbs:15:12 ReferenceError: Can't find variable: includePaths")); } +void TestBlackbox::removeDuplicateLibraries_data() +{ + QTest::addColumn("removeDuplicates"); + QTest::newRow("remove duplicates") << true; + QTest::newRow("don't remove duplicates") << false; +} + +void TestBlackbox::removeDuplicateLibraries() +{ + QDir::setCurrent(testDataDir + "/remove-duplicate-libs"); + QFETCH(bool, removeDuplicates); + const QbsRunParameters resolveParams("resolve", {"-f", "remove-duplicate-libs.qbs", + "project.removeDuplicates:" + QString(removeDuplicates? "true" : "false")}); + QCOMPARE(runQbs(resolveParams), 0); + const bool isBfd = m_qbsStdout.contains("is bfd linker: true"); + const bool isNotBfd = m_qbsStdout.contains("is bfd linker: false"); + QVERIFY2(isBfd != isNotBfd, m_qbsStdout.constData()); + QbsRunParameters buildParams("build"); + buildParams.expectFailure = removeDuplicates && isBfd; + QCOMPARE(runQbs(buildParams) == 0, !buildParams.expectFailure); +} + void TestBlackbox::reproducibleBuild() { const SettingsPtr s = settings(); @@ -4457,6 +4479,86 @@ void TestBlackbox::lexyaccOutputs_data() << QString() << QString{"shaven_yak.cpp"}; } +void TestBlackbox::linkerLibraryDuplicates() +{ + const SettingsPtr s = settings(); + Profile buildProfile(profileName(), s.get()); + QStringList toolchain = buildProfile.value("qbs.toolchain").toStringList(); + if (!toolchain.contains("gcc")) + QSKIP("linkerLibraryDuplicates test only applies to GCC toolchain"); + + QDir::setCurrent(testDataDir + "/linker-library-duplicates"); + rmDirR(relativeBuildDir()); + + QFETCH(QString, removeDuplicateLibraries); + QStringList runParams; + if (!removeDuplicateLibraries.isEmpty()) { + runParams.append(removeDuplicateLibraries); + } + + QCOMPARE(runQbs(QbsRunParameters("resolve", runParams)), 0); + QCOMPARE(runQbs(QStringList { "--command-echo-mode", "command-line" }), 0); + const QByteArrayList output = m_qbsStdout.split('\n'); + QByteArray linkLine; + for (const QByteArray &line : output) { + if (line.contains("main.cpp.o")) + linkLine = line; + } + QVERIFY(!linkLine.isEmpty()); + + /* Now check the the libraries appear just once. In order to avoid dealing + * with the different file extensions used in different platforms, we check + * only for the library base name. But we must also take into account that + * the build directories of each library will contain the library base name, + * so we now exclude them. */ + QByteArrayList elementsWithoutPath; + for (const QByteArray &element: linkLine.split(' ')) { + if (element.indexOf('/') < 0) + elementsWithoutPath.append(element); + } + QByteArray pathLessLinkLine = elementsWithoutPath.join(' '); + + typedef QMap ObjectCount; + QFETCH(ObjectCount, expectedObjectCount); + for (auto i = expectedObjectCount.begin(); + i != expectedObjectCount.end(); + i++) { + QCOMPARE(pathLessLinkLine.count(i.key()), i.value()); + } +} + +void TestBlackbox::linkerLibraryDuplicates_data() +{ + typedef QMap ObjectCount; + + QTest::addColumn("removeDuplicateLibraries"); + QTest::addColumn("expectedObjectCount"); + + QTest::newRow("default") << + QString() << + ObjectCount { + { "lib1", 1 }, + { "lib2", 1 }, + { "lib3", 1 }, + }; + + QTest::newRow("enabled") << + "modules.cpp.removeDuplicateLibraries:true" << + ObjectCount { + { "lib1", 1 }, + { "lib2", 1 }, + { "lib3", 1 }, + }; + + QTest::newRow("disabled") << + "modules.cpp.removeDuplicateLibraries:false" << + ObjectCount { + { "lib1", 3 }, + { "lib2", 2 }, + { "lib3", 1 }, + }; +} + void TestBlackbox::linkerScripts() { const SettingsPtr s = settings(); diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h index 3a66b6962..4e2755724 100644 --- a/tests/auto/blackbox/tst_blackbox.h +++ b/tests/auto/blackbox/tst_blackbox.h @@ -171,6 +171,8 @@ private slots: void lexyacc(); void lexyaccOutputs(); void lexyaccOutputs_data(); + void linkerLibraryDuplicates(); + void linkerLibraryDuplicates_data(); void linkerScripts(); void listProducts(); void listPropertiesWithOuter(); @@ -249,6 +251,8 @@ private slots: void recursiveRenaming(); void recursiveWildcards(); void referenceErrorInExport(); + void removeDuplicateLibraries_data(); + void removeDuplicateLibraries(); void reproducibleBuild(); void reproducibleBuild_data(); void require(); -- cgit v1.2.3