Fix Sparkle 2 build error on older Xcode / fix sign/notarize scripts

Sparkle 2:

Seems like older Xcode versions (before 12.5) have problems signing
frameworks that only have a Versions/B but not a Versions/A folder.
Sparkle 2 moved to only having Versions/B which is causing code sign to
fail on build step. Just fix this issue by disabling Sign on Copy for
Sparkle.framework. We don't really need it because it already comes with
an ad-hoc signature anyway, and for publish we have a custom signing
script (sign-developer-id) to do manual signing.

Also, fix CI to use Sparkle 2 again for old Xcode (11.7) builds to test
this working. Previously we made it use Sparkle 1 because we didn't work
around this issue, which was kind of a hack.

For Sparkle 2, also add a `cleanup-after-build` script to remove the XPC
Services folder in the framework. Sparkle dev added this to the bundle
for sandboxed apps but non-sandbox apps don't really need it and it's
recommended by Sparkle to remove them, so we unfortunately need to
manually do it as a post-build step.

Fix #1335

Signing / notarization scripts:

Make sure to sign Sparkle 2 binaries in `sign-developer-id`. Also, seems
like the "--deep" flag in codesign is now deprecated as of macOS 13, so
just stop using it and manually sign all the relevant binaries
explicitly.

For notarization script, the current behavior doesn't correctly detect
failure as `xcrun notarytool submit` always exits with 0 even if the
submission failed. Add logic to manually query whether the submission
succeeded. If failed, print out the logs for easier diagnosis, and
exits with -1 so it will block progress.

Author: ychin

.github/workflows/ci-macvim.yaml

@@ -78,14 +78,8 @@ jobs:
           echo "MACOSX_DEPLOYMENT_TARGET=$MACOSX_DEPLOYMENT_TARGET_LEGACY" >> $GITHUB_ENV
           echo "MACVIM_ARCHS=$MACVIM_ARCHS_LEGACY" >> $GITHUB_ENV
 
-      - name: Set up Sparkle 1
-        if: matrix.legacy || !matrix.publish
-        run: |
           # Use Sparkle 1 because Sparkle 2 requires newer OS version than our legacy build.
           # Later, we pass the --enable-sparkle_1 flag to configure to set the corresponding ifdef.
-          #
-          # We also do this for non-publish builds, because those are usually run on older versions
-          # of Xcode, and they cannot handle the Sparkle 2 framework as it's built using newer Xcode.
           ln -fhs Sparkle_1.framework src/MacVim/Sparkle.framework
 
       # Set up, install, and cache gettext library for localization.

src/MacVim/MacVim.xcodeproj/project.pbxproj

@@ -65,7 +65,7 @@
 		52818B031C1C08CE00F59085 /* QLStephen.qlgenerator in Copy QuickLookPlugin */ = {isa = PBXBuildFile; fileRef = 52818AFF1C1C075300F59085 /* QLStephen.qlgenerator */; settings = {ATTRIBUTES = (CodeSignOnCopy, ); }; };
 		528DA66A1426D4F9003380F1 /* macvim-askpass in Copy Scripts */ = {isa = PBXBuildFile; fileRef = 528DA6691426D4EB003380F1 /* macvim-askpass */; };
 		52A364731C4A5789005757EC /* Sparkle.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 52A364721C4A5789005757EC /* Sparkle.framework */; settings = {ATTRIBUTES = (Weak, ); }; };
-		52A364761C4A57C1005757EC /* Sparkle.framework in Copy Frameworks */ = {isa = PBXBuildFile; fileRef = 52A364721C4A5789005757EC /* Sparkle.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
+		52A364761C4A57C1005757EC /* Sparkle.framework in Copy Frameworks */ = {isa = PBXBuildFile; fileRef = 52A364721C4A5789005757EC /* Sparkle.framework */; settings = {ATTRIBUTES = (RemoveHeadersOnCopy, ); }; };
 		8D11072B0486CEB800E47090 /* InfoPlist.strings in Resources */ = {isa = PBXBuildFile; fileRef = 089C165CFE840E0CC02AAC07 /* InfoPlist.strings */; };
 		8D11072D0486CEB800E47090 /* main.m in Sources */ = {isa = PBXBuildFile; fileRef = 29B97316FDCFA39411CA2CEA /* main.m */; settings = {ATTRIBUTES = (); }; };
 		8D11072F0486CEB800E47090 /* Cocoa.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 1058C7A1FEA54F0111CA2CBB /* Cocoa.framework */; };
@@ -870,6 +870,7 @@
 				528DA6671426D456003380F1 /* Copy Scripts */,
 				52283AB71EBA200C00A6F6B9 /* Copy mvim scripts */,
 				90C052E1251E889500E2D81E /* Copy vimtutor */,
+				9099B3CA2914B272005C9F32 /* Clean up build */,
 			);
 			buildRules = (
 			);
@@ -1035,6 +1036,26 @@
 			shellScript = "BINPATH=$BUILT_PRODUCTS_DIR/$CONTENTS_FOLDER_PATH/bin\nmkdir -p \"${BINPATH}\"\ncp -a \"${SRCROOT}/mvim\" \"${BINPATH}/mvim\"\nchmod 755 \"${BINPATH}/mvim\"\n(cd \"${BINPATH}\"; for f in vim vimdiff view gvim gvimdiff gview mvimdiff mview; do ln -fs mvim $f; done)\n";
 			showEnvVarsInLog = 0;
 		};
+		9099B3CA2914B272005C9F32 /* Clean up build */ = {
+			isa = PBXShellScriptBuildPhase;
+			alwaysOutOfDate = 1;
+			buildActionMask = 2147483647;
+			files = (
+			);
+			inputFileListPaths = (
+			);
+			inputPaths = (
+			);
+			name = "Clean up build";
+			outputFileListPaths = (
+			);
+			outputPaths = (
+			);
+			runOnlyForDeploymentPostprocessing = 0;
+			shellPath = /bin/sh;
+			shellScript = "./scripts/cleanup-after-build $BUILT_PRODUCTS_DIR/$WRAPPER_NAME\n";
+			showEnvVarsInLog = 0;
+		};
 		90BD4EF224E0E8B700BF29F2 /* Copy locale message translation files */ = {
 			isa = PBXShellScriptBuildPhase;
 			buildActionMask = 2147483647;

src/MacVim/scripts/cleanup-after-build

@@ -0,0 +1,22 @@
+#!/bin/sh
+
+# Utility script to clean up after a MacVim build.
+
+if [[ $# == 0 ]]; then
+    echo "Usage: cleanup-after-build <MacVim_app>"
+    exit -1
+fi
+
+set -e
+
+macvim_path=$1
+
+sparkle_xpcservices="$macvim_path/Contents/Frameworks/Sparkle.framework/Versions/Current/XPCServices"
+
+if [ -d $sparkle_xpcservices ]; then
+    # This only happens when building using Sparkle 2. It contains XPC Services
+    # files which are only necessary for sandboxed apps, and not recommended
+    # otherwise. See https://sparkle-project.org/documentation/sandboxing/.
+    set -x
+    rm -rf $sparkle_xpcservices
+fi

src/MacVim/scripts/notarize-dmg

@@ -30,10 +30,38 @@ if [[ "$USE_ALTOOL" != "1" ]]; then
         exit -1
     fi
 
-    # Step 1: Submit app for notarization, and wait till it finishes.
-    set -x
-    xcrun notarytool submit ${macvim_dmg} --keychain-profile "${NOTARIZATION_KEYCHAIN_PROFILE}" --progress --wait --timeout=20m
-    set +x
+    # Step 1.1: Submit app for notarization, and wait till it finishes.
+    notarize_submit_results=$(set -x; xcrun notarytool submit ${macvim_dmg} --keychain-profile "${NOTARIZATION_KEYCHAIN_PROFILE}")
+
+    echo $notarize_submit_results
+    notarize_submit_uuid=$(echo $notarize_submit_results | grep "id:" | head -1 | sed -E "s/^.*id:[ ]*(.*)/\1/")
+
+    if [[ ${notarize_submit_uuid} == "" ]]; then
+        echo "Failed to submit for notarization!"
+        exit -1
+    fi
+    if ! [[ ${notarize_submit_uuid} =~ "^[a-f0-9\-]*$" ]]; then
+        echo "Request UUID format error!"
+        echo ${notarize_submit_uuid}
+        exit -1
+    fi
+
+    (set -x; xcrun notarytool wait --keychain-profile "${NOTARIZATION_KEYCHAIN_PROFILE}" --progress --timeout=20m $notarize_submit_uuid)
+
+    # Step 1.2: Confirm that notarization succeeded. If not, print the log. We
+    # have to manually query instead of using return code because xcrun
+    # notarytool always exits with 0 regardless of success.
+    notarize_results=$(xcrun notarytool info --keychain-profile "${NOTARIZATION_KEYCHAIN_PROFILE}" $notarize_submit_uuid)
+    notarize_status=$(echo $notarize_results | grep "status:" | sed -E "s/^.*status:[ ]*(.*)/\1/")
+    if [[ $notarize_status == "Accepted" ]]; then
+        printf "\n"
+        echo "Notarization Success!\n"
+    else
+        printf "\n"
+        echo "Notarization Failed!\n"
+        xcrun notarytool log --keychain-profile "${NOTARIZATION_KEYCHAIN_PROFILE}" $notarize_submit_uuid
+        exit -1
+    fi
 else
     # Legacy Xcode 12 path using altool and username + app-specific password.
     # See: https://developer.apple.com/documentation/security/notarizing_macos_software_before_distribution/customizing_the_notarization_workflow/notarizing_apps_when_developing_with_xcode_12_and_earlier
@@ -89,13 +117,10 @@ fi
 
 # Step 2: Staple the notarization info to the DMG so that an offline user can
 # verify that it is notarized.
-set -x
-xcrun stapler staple ${macvim_dmg}
-set +x
+(set -x; xcrun stapler staple ${macvim_dmg})
 
 # Just print out extra info for reference
 echo "--------------------"
 set -x
 codesign -d --verbose=2 ${macvim_dmg}
 spctl -a -t open --context context:primary-signature -v ${macvim_dmg}
-set +x

src/MacVim/scripts/sign-developer-id

@@ -21,11 +21,23 @@ if [[ $macvim_path =~ dmg ]]; then
     set -x
     codesign -f -s "Developer ID Application" -o runtime --timestamp $macvim_path
 else
-    # Sign bottom-up to make sure everything is signed. Note: --deep doesn't
-    # catch certain edge cases like the files in Resources, hence the need to
-    # manually sign them before signing the main app.
+    # Sign bottom-up to make sure everything is signed in order.
+    # Note: Not using --deep because it's been deprecated since macOS 13, and
+    # also it doesn't catch all the binaries anyway so it's better to just be
+    # explicit and sign everything in order to be clear what we are doing.
+    if [ -d $macvim_path/Contents/Frameworks/Sparkle.framework/Versions/A ]; then
+        (set -x
+        codesign -f -s "Developer ID Application" -o runtime --timestamp $macvim_path/Contents/Frameworks/Sparkle.framework/Versions/A/Resources/Autoupdate.app)
+    fi
+    if [ -d $macvim_path/Contents/Frameworks/Sparkle.framework/Versions/B ]; then
+        (set -x
+        codesign -f -s "Developer ID Application" -o runtime --timestamp $macvim_path/Contents/Frameworks/Sparkle.framework/Versions/B/Autoupdate
+        codesign -f -s "Developer ID Application" -o runtime --timestamp $macvim_path/Contents/Frameworks/Sparkle.framework/Versions/B/Updater.app)
+    fi
     set -x
-    codesign -f -s "Developer ID Application" -o runtime --deep --timestamp $macvim_path/Contents/Frameworks/Sparkle.framework/Versions/A/Resources/Autoupdate.app
-    codesign -f -s "Developer ID Application" -o runtime --deep --timestamp $macvim_path/Contents/Library/QuickLook/QLStephen.qlgenerator/Contents/MacOS/QLStephen
-    codesign -f -s "Developer ID Application" -o runtime --deep --timestamp --entitlements $entitlements $macvim_path
+    codesign -f -s "Developer ID Application" -o runtime --timestamp $macvim_path/Contents/Frameworks/Sparkle.framework
+    codesign -f -s "Developer ID Application" -o runtime --timestamp $macvim_path/Contents/Frameworks/PSMTabBarControl.framework
+    codesign -f -s "Developer ID Application" -o runtime --timestamp $macvim_path/Contents/Library/QuickLook/QLStephen.qlgenerator/Contents/MacOS/QLStephen
+    codesign -f -s "Developer ID Application" -o runtime --timestamp --entitlements $entitlements $macvim_path/Contents/MacOS/Vim
+    codesign -f -s "Developer ID Application" -o runtime --timestamp --entitlements $entitlements $macvim_path
 fi