GCC: remove escaped linker flags compatibility mode

The old warning was incredibly noisy and almost always unnecessary.
Users should have had enough time to migrate their projects by now, so
specifying pre-escaped linker flags will now properly result in a linker
error, but a helpful warning remains by default, which is only emitted
if the linker flags look suspicious to begin with.

Change-Id: I1ad1e9647fd490619eeb65eeb88f5bbbf58aa18f
Reviewed-by: Jake Petroules <jake.petroules@qt.io>

7 files changed, 51 insertions, 17 deletions

diff --git a/share/qbs/modules/cpp/CppModule.qbs b/share/qbs/modules/cpp/CppModule.qbs
index 3575683a8..8742e618e 100644
--- a/share/qbs/modules/cpp/CppModule.qbs
+++ b/share/qbs/modules/cpp/CppModule.qbs
@@ -43,6 +43,7 @@ Module {
     property int compilerVersionPatch
     property string warningLevel : 'all' // 'none', 'all'
     property bool treatWarningsAsErrors : false
+    property bool enableSuspiciousLinkerFlagWarnings: true
     property string architecture: qbs.architecture
     property string machineType // undocumented
     property string imageFormat // undocumented
diff --git a/share/qbs/modules/cpp/gcc.js b/share/qbs/modules/cpp/gcc.js
index e6377dcec..7c6108870 100644
--- a/share/qbs/modules/cpp/gcc.js
+++ b/share/qbs/modules/cpp/gcc.js
@@ -173,14 +173,11 @@ function collectLibraryDependencies(product) {
     return result;
 }
 
-function escapeLinkerFlags(product, inputs, linkerFlags, allowEscape) {
-    if (allowEscape === undefined)
-        allowEscape = true;
-
+function escapeLinkerFlags(product, inputs, linkerFlags) {
     if (!linkerFlags || linkerFlags.length === 0)
         return [];
 
-    if (useCompilerDriverLinker(product, inputs) && allowEscape) {
+    if (useCompilerDriverLinker(product, inputs)) {
         var sep = ",";
         var useXlinker = linkerFlags.some(function (f) { return f.contains(sep); });
         if (useXlinker) {
@@ -188,11 +185,23 @@ function escapeLinkerFlags(product, inputs, linkerFlags, allowEscape) {
             // Use -Xlinker to handle these
             var xlinkerFlags = [];
             linkerFlags.map(function (linkerFlag) {
+                if (product.cpp.enableSuspiciousLinkerFlagWarnings
+                        && linkerFlag.startsWith("-Wl,")) {
+                    console.warn("Encountered escaped linker flag '" + linkerFlag + "'. This may " +
+                                 "cause the target to fail to link. Please do not escape these " +
+                                 "flags manually; qbs does that for you.");
+                }
                 xlinkerFlags.push("-Xlinker", linkerFlag);
             });
             return xlinkerFlags;
         }
 
+        if (product.cpp.enableSuspiciousLinkerFlagWarnings && linkerFlags.contains("-Xlinker")) {
+            console.warn("Encountered -Xlinker linker flag escape sequence. This may cause the " +
+                         "target to fail to link. Please do not escape these flags manually; " +
+                         "qbs does that for you.");
+        }
+
         // If no linker arguments contain the separator character we can just use -Wl,
         // which is more compact and easier to read in logs
         return [["-Wl"].concat(linkerFlags).join(sep)];
@@ -335,19 +344,10 @@ function linkerFlags(project, product, inputs, output) {
     if (isDarwin && product.cpp.warningLevel === "none")
         args.push('-w');
 
-    var allowEscape = !ModUtils.checkCompatibilityMode(project, "1.6",
-        "Enabling linker flags compatibility mode. cpp.linkerFlags and " +
-        "cpp.platformLinkerFlags escaping is handled automatically beginning in Qbs 1.6. " +
-        "When upgrading to Qbs 1.6, you should only pass raw linker flags to these " +
-        "properties; do not escape them using -Wl or -Xlinker. This allows Qbs to " +
-        "automatically supply the correct linker flags regardless of whether the " +
-        "linker chosen is the compiler driver or system linker (see the documentation for " +
-        "cpp.linkerMode for more information).");
-
     args = args.concat(configFlags(product, useCompilerDriverLinker(product, inputs)));
     args = args.concat(escapeLinkerFlags(
-                           product, inputs, product.cpp.platformLinkerFlags, allowEscape));
-    args = args.concat(escapeLinkerFlags(product, inputs, product.cpp.linkerFlags, allowEscape));
+                           product, inputs, product.cpp.platformLinkerFlags));
+    args = args.concat(escapeLinkerFlags(product, inputs, product.cpp.linkerFlags));
 
     args.push("-o", output.filePath);
 
diff --git a/tests/auto/api/testdata/static-lib-deps/static-lib-deps.qbs b/tests/auto/api/testdata/static-lib-deps/static-lib-deps.qbs
index 074acc8a5..ccbe49a0b 100644
--- a/tests/auto/api/testdata/static-lib-deps/static-lib-deps.qbs
+++ b/tests/auto/api/testdata/static-lib-deps/static-lib-deps.qbs
@@ -94,7 +94,7 @@ Project {
 
         Properties {
             condition: qbs.targetOS.contains("linux")
-            cpp.linkerFlags: ["-static"]
+            cpp.driverFlags: ["-static"]
         }
 
         files: [
diff --git a/tests/auto/blackbox/testdata/escaped-linker-flags/escaped-linker-flags.qbs b/tests/auto/blackbox/testdata/escaped-linker-flags/escaped-linker-flags.qbs
new file mode 100644
index 000000000..b93c41cd7
--- /dev/null
+++ b/tests/auto/blackbox/testdata/escaped-linker-flags/escaped-linker-flags.qbs
@@ -0,0 +1,15 @@
+import qbs
+
+CppApplication {
+    name: "app"
+    property bool escapeLinkerFlags
+    Properties {
+        condition: escapeLinkerFlags
+        cpp.linkerFlags: ["-Wl,-z,defs"]
+    }
+    Properties {
+        condition: !escapeLinkerFlags
+        cpp.linkerFlags: ["-z", "defs"]
+    }
+    files: ["main.cpp"]
+}
diff --git a/tests/auto/blackbox/testdata/escaped-linker-flags/main.cpp b/tests/auto/blackbox/testdata/escaped-linker-flags/main.cpp
new file mode 100644
index 000000000..8b8d58de0
--- /dev/null
+++ b/tests/auto/blackbox/testdata/escaped-linker-flags/main.cpp
@@ -0,0 +1 @@
+int main() { }
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index 3e40e18ce..fe405a502 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -2712,6 +2712,22 @@ void TestBlackbox::errorInfo()
     QVERIFY2(m_qbsStderr.contains("error-info.qbs:58"), m_qbsStderr);
 }
 
+void TestBlackbox::escapedLinkerFlags()
+{
+    Settings settings((QString()));
+    const Profile buildProfile(profileName(), &settings);
+    const QStringList toolchain = buildProfile.value("qbs.toolchain").toStringList();
+    if (!toolchain.contains("gcc") || targetOs() == HostOsInfo::HostOsMacos)
+        QSKIP("escaped linker flags test only applies with gcc and GNU ld");
+    QDir::setCurrent(testDataDir + "/escaped-linker-flags");
+    QbsRunParameters params(QStringList("products.app.escapeLinkerFlags:false"));
+    QCOMPARE(runQbs(params), 0);
+    params.arguments = QStringList() << "products.app.escapeLinkerFlags:true";
+    params.expectFailure = true;
+    QVERIFY(runQbs(params) != 0);
+    QVERIFY2(m_qbsStderr.contains("Encountered escaped linker flag"), m_qbsStderr.constData());
+}
+
 void TestBlackbox::systemRunPaths()
 {
     Settings settings((QString()));
diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h
index fed051edf..a11f87343 100644
--- a/tests/auto/blackbox/tst_blackbox.h
+++ b/tests/auto/blackbox/tst_blackbox.h
@@ -80,6 +80,7 @@ private slots:
     void erroneousFiles_data();
     void erroneousFiles();
     void errorInfo();
+    void escapedLinkerFlags();
     void exportRule();
     void exportToOutsideSearchPath();
     void fileDependencies();