apple: fix codesign module

... when multiplexing over build variants.

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

4 files changed, 98 insertions, 54 deletions

diff --git a/changelogs/changes-2.3.1.md b/changelogs/changes-2.3.1.md
new file mode 100644
index 000000000..0514bc204
--- /dev/null
+++ b/changelogs/changes-2.3.1.md
@@ -0,0 +1,8 @@
+# Apple support
+* Fixed codesing module when multiplexing over build variants (QBS-1775).
+
+# CI
+* Fixed release jobs.
+
+# Contributors
+* Ivan Komissarov
diff --git a/share/qbs/modules/codesign/codesign.js b/share/qbs/modules/codesign/codesign.js
index 903d16f80..482225ea2 100644
--- a/share/qbs/modules/codesign/codesign.js
+++ b/share/qbs/modules/codesign/codesign.js
@@ -280,20 +280,21 @@ function prepareSign(project, product, inputs, outputs, input, output) {
         return cmds;
 
     var isBundle = "bundle.content" in outputs;
-    var outputFilePath = isBundle
-            ? FileInfo.joinPaths(product.destinationDirectory, product.bundle.bundleName)
-            : outputs["codesign.signed_artifact"][0].filePath;
-    var outputFileName = isBundle
-            ? product.bundle.bundleName
-            : outputs["codesign.signed_artifact"][0].fileName;
-    var isProductBundle = product.bundle && product.bundle.isBundle;
 
-    // If the product is a bundle, just sign the bundle
-    // instead of signing the bundle and executable separately
+    var artifacts = [];
+    if (isBundle) {
+        artifacts = [{
+            filePath: FileInfo.joinPaths(product.destinationDirectory, product.bundle.bundleName),
+            fileName: product.bundle.bundleName
+        }];
+    } else {
+        artifacts = outputs["codesign.signed_artifact"];
+    }
+    var isProductBundle = product.bundle && product.bundle.isBundle;
     var shouldSignArtifact = !isProductBundle || isBundle;
 
     var enableCodeSigning = product.codesign.enableCodeSigning;
-    if (enableCodeSigning && shouldSignArtifact) {
+    if (enableCodeSigning) {
         var actualSigningIdentity = product.codesign._actualSigningIdentity;
         if (!actualSigningIdentity) {
             throw "No codesigning identities (i.e. certificate and private key pairs) matching “"
@@ -310,36 +311,53 @@ function prepareSign(project, product, inputs, outputs, input, output) {
             }
         }
 
-        var args = ["--force", "--sign", actualSigningIdentity.SHA1];
-
-        // If signingTimestamp is undefined or empty, do not specify the flag at all -
-        // this uses the system-specific default behavior
-        var signingTimestamp = product.codesign.signingTimestamp;
-        if (signingTimestamp) {
-            // If signingTimestamp is an empty string, specify the flag but do
-            // not specify a value - this uses a default Apple-provided server
-            var flag = "--timestamp";
-            if (signingTimestamp)
-                flag += "=" + signingTimestamp;
-            args.push(flag);
+        // The codesign tool behaves weirdly. It can sign a bundle with a single artifact, but if
+        // say debug build variant is present, it starts complaining that it is not signed.
+        // We could always sign everything, but again, in case of a framework (but not in case of
+        // app or loadable bundle), codesign produces a warning that artifact is already signed.
+        // So, we skip signing the release artifact and only sign if other build variants present.
+        if (!shouldSignArtifact && artifacts.length == 1) {
+            artifacts = [];
         }
+        for (var i = 0; i < artifacts.length; ++i) {
+            if (!shouldSignArtifact
+                && artifacts[i].qbs && artifacts[i].qbs.buildVariant === "release") {
+                continue;
+            }
+            var outputFilePath = artifacts[i].filePath;
+            var outputFileName = artifacts[i].fileName;
+
+            var args = ["--force", "--sign", actualSigningIdentity.SHA1];
+
+            // If signingTimestamp is undefined or empty, do not specify the flag at all -
+            // this uses the system-specific default behavior
+            var signingTimestamp = product.codesign.signingTimestamp;
+            if (signingTimestamp) {
+                // If signingTimestamp is an empty string, specify the flag but do
+                // not specify a value - this uses a default Apple-provided server
+                var flag = "--timestamp";
+                if (signingTimestamp)
+                    flag += "=" + signingTimestamp;
+                args.push(flag);
+            }
 
-        for (var j in inputs["codesign.xcent"]) {
-            args.push("--entitlements", inputs["codesign.xcent"][j].filePath);
-            break; // there should only be one
-        }
+            for (var j in inputs["codesign.xcent"]) {
+                args.push("--entitlements", inputs["codesign.xcent"][j].filePath);
+                break; // there should only be one
+            }
 
-        args = args.concat(product.codesign.codesignFlags || []);
+            args = args.concat(product.codesign.codesignFlags || []);
 
-        args.push(outputFilePath + subpath);
-        cmd = new Command(product.codesign.codesignPath, args);
-        cmd.description = "codesign " + outputFileName
-                + " (" + actualSigningIdentity.subjectInfo.CN + ")";
-        cmd.outputFilePath = outputFilePath;
-        cmd.stderrFilterFunction = function(stderr) {
-            return stderr.replace(outputFilePath + ": replacing existing signature\n", "");
-        };
-        cmds.push(cmd);
+            args.push(outputFilePath + subpath);
+            cmd = new Command(product.codesign.codesignPath, args);
+            cmd.description = "codesign " + outputFileName
+                    + " (" + actualSigningIdentity.subjectInfo.CN + ")";
+            cmd.outputFilePath = outputFilePath;
+            cmd.stderrFilterFunction = function(stderr) {
+                return stderr.replace(outputFilePath + ": replacing existing signature\n", "");
+            };
+            cmds.push(cmd);
+        }
     }
 
     if (isBundle) {
diff --git a/tests/auto/blackbox/testdata-apple/codesign/codesign.qbs b/tests/auto/blackbox/testdata-apple/codesign/codesign.qbs
index 08c8f730b..c1fc0502a 100644
--- a/tests/auto/blackbox/testdata-apple/codesign/codesign.qbs
+++ b/tests/auto/blackbox/testdata-apple/codesign/codesign.qbs
@@ -2,10 +2,13 @@ import "../multiarch-helpers.js" as Helpers
 
 Project {
     name: "p"
+    // we do not have the access to xcode version in qbs.architectures so we need to pass it here
     property string xcodeVersion
 
     property bool isBundle: true
     property bool enableSigning: true
+    property bool multiArch: false
+    property bool multiVariant: false
 
     CppApplication {
         name: "A"
@@ -18,7 +21,8 @@ Project {
         installDir: ""
 
         qbs.architectures:
-            project.xcodeVersion ? Helpers.getArchitectures(qbs, project.xcodeVersion) : []
+            multiArch ? Helpers.getArchitectures(qbs, project.xcodeVersion) : []
+        qbs.buildVariants: project.multiVariant ? ["debug", "release"] : []
     }
 
     DynamicLibrary {
@@ -32,7 +36,8 @@ Project {
         install: true
         installDir: ""
         qbs.architectures:
-            project.xcodeVersion ? Helpers.getArchitectures(qbs, project.xcodeVersion) : []
+            multiArch ? Helpers.getArchitectures(qbs, project.xcodeVersion) : []
+        qbs.buildVariants: project.multiVariant ? ["debug", "release"] : []
     }
 
     LoadableModule {
@@ -46,6 +51,7 @@ Project {
         install: true
         installDir: ""
         qbs.architectures:
-            project.xcodeVersion ? Helpers.getArchitectures(qbs, project.xcodeVersion) : []
+            multiArch ? Helpers.getArchitectures(qbs, project.xcodeVersion) : []
+        qbs.buildVariants: project.multiVariant ? ["debug", "release"] : []
     }
 }
diff --git a/tests/auto/blackbox/tst_blackboxapple.cpp b/tests/auto/blackbox/tst_blackboxapple.cpp
index 02b56f603..30e20a1c9 100644
--- a/tests/auto/blackbox/tst_blackboxapple.cpp
+++ b/tests/auto/blackbox/tst_blackboxapple.cpp
@@ -712,9 +712,11 @@ void TestBlackboxApple::byteArrayInfoPlist()
 
 void TestBlackboxApple::codesign()
 {
+    QFETCH(int, expectedCount);
     QFETCH(bool, isBundle);
     QFETCH(bool, enableSigning);
     QFETCH(bool, multiArch);
+    QFETCH(bool, multiVariant);
 
     const auto xcodeVersion = findXcodeVersion();
 
@@ -723,20 +725,21 @@ void TestBlackboxApple::codesign()
 
     QDir::setCurrent(testDataDir + "/codesign");
     QbsRunParameters params(QStringList{"qbs.installPrefix:''"});
-    params.arguments
-            << QStringLiteral("project.isBundle:%1").arg(isBundle ? "true" : "false");
-    params.arguments
-            << QStringLiteral("project.enableSigning:%1").arg(enableSigning ? "true" : "false");
-    if (multiArch)
-        params.arguments << QStringLiteral("project.xcodeVersion:") + xcodeVersion->toString();
+    // the test can't use xcode module to determine version itself
+    params.arguments << QStringLiteral("project.xcodeVersion:") + xcodeVersion->toString();
+    params.arguments << QStringLiteral("project.isBundle:%1").arg(isBundle ? "true" : "false");
+    params.arguments << QStringLiteral("project.enableSigning:%1")
+                            .arg(enableSigning ? "true" : "false");
+    params.arguments << QStringLiteral("project.multiArch:%1").arg(multiArch ? "true" : "false");
+    params.arguments << QStringLiteral("project.multiVariant:%1")
+                            .arg(multiVariant ? "true" : "false");
 
     rmDirR(relativeBuildDir());
     QCOMPARE(runQbs(params), 0);
 
     const int codeSignCount =
             QString::fromUtf8(m_qbsStdout).count(QStringLiteral("codesign"));
-    // We have 3 products, we have to sign each exactly once, even in the multiplexed case
-    QCOMPARE(codeSignCount, enableSigning ? 3 : 0);
+    QCOMPARE(codeSignCount, expectedCount);
 
     const auto appName = isBundle ? QStringLiteral("A.app") : QStringLiteral("A");
     const auto appPath = defaultInstallRoot + "/" + appName;
@@ -781,16 +784,25 @@ void TestBlackboxApple::codesign()
 
 void TestBlackboxApple::codesign_data()
 {
+    QTest::addColumn<int>("expectedCount");
     QTest::addColumn<bool>("isBundle");
     QTest::addColumn<bool>("enableSigning");
     QTest::addColumn<bool>("multiArch");
-
-    QTest::newRow("bundle, unsigned") << true << false << false;
-    QTest::newRow("standalone, unsigned") << false << false << false;
-    QTest::newRow("bundle, signed") << true << true << false;
-    QTest::newRow("standalone, signed") << false << true << false;
-    QTest::newRow("bundle, signed, multiarch") << true << true << true;
-    QTest::newRow("standalone, signed, multiarch") << false << true << true;
+    QTest::addColumn<bool>("multiVariant");
+
+    QTest::newRow("standalone, unsigned") << 0 << false << false << false << false;
+    QTest::newRow("bundle, unsigned") << 0 << true << false << false << false;
+    QTest::newRow("standalone, signed") << 3 << false << true << false << false;
+    QTest::newRow("bundle, signed") << 3 << true << true << false << false;
+    // here we only sign the resulting lipo artifact
+    QTest::newRow("standalone, signed, multiarch") << 3 << false << true << true << false;
+    QTest::newRow("bundle, signed, multiarch") << 3 << true << true << true << false;
+    // here we sign all artifacts
+    QTest::newRow("standalone, signed, multivariant") << 15 << false << true << false << true;
+    QTest::newRow("bundle, signed, multivariant") << 15 << true << true << false << true;
+    QTest::newRow("standalone, signed, multiarch, multivariant")
+        << 15 << false << true << true << true;
+    QTest::newRow("bundle, signed, multiarch, multivariant") << 15 << true << true << true << true;
 }
 
 void TestBlackboxApple::deploymentTarget()