Remove Sparkle.framework when configured with --disable-sparkle

Previously, when configured with `--disable-sparkle`, the
Sparkle.framework would still get linked and copied into the app bundle.
This is because Xcode doesn't have an easy way to disable individual
build phase. We would either have to make a new build target, or
dynamically patch the project file in order for that to not happen.
Package managers like Homebrew and Nix have to either manually delete
the framework as a post-build step, or manually patch the pbxproj file
as a pre-build step to make sure the framework never got linked/copied.

Also, when building MacVim on macOS 10.9-10.12, where Sparkle 2 is not
supported, even if you build with `--disable-sparkle`, the app still
wouldn't run since Sparkle 2's framework still got copied into the app
and on launch the OS will detect that it's not compatible with the OS
version.

To fix this, just add a new env var and have the build cleanup script
delete the entire Sparkle.framework as a post-build step, if
`--disable-sparkle` was previously configured.

One thing to note is that currently, even with this, the MacVim binary
still has a weak linking dependency to the non-existent
Sparkle.framework (can be seen by doing `otool -L
MacVim.app/Contents/MacOS/MacVim`) but it should be fine.  A proper way
to fix this is to manually link Sparkle instead of letting Xcode do it
for us, but considering that this is a relatively niche use case, and it
still works regardless, there isn't a pressing need to do so.

Also, add CI check to make sure when `--disable-sparkle` is used, the
actual MacVim binary has 0 references to Sparkle symbols. This makes
sure that removing the Sparkle.framework from the app bundle is actually
safe.

.github/workflows/ci-macvim.yaml

@@ -242,6 +242,26 @@ jobs:
           ${VIM_BIN} -u NONE -i NONE --not-a-term -esNX -V1 -S ci/if_ver-2.vim -c quit
 
       - name: Smoketest
+        run: |
+          set -o verbose
+
+          # Make sure there isn't any dynamic linkage to third-party dependencies in the built binary, as we should only use
+          # static linkage to avoid dependency hell. Test that all those dylib's are in /usr/lib which is bundled with macOS and not third-party.
+          if otool -L ${VIM_BIN} | grep '\.dylib\s' | grep -v '^\s*/usr/lib/'; then
+            echo 'Found external dynamic linkage!'; false
+          fi
+
+          # Make sure that --disable-sparkle flag will properly exclude all references to Sparkle symbols. This is
+          # necessary because we still use weak linking to Sparkle when that flag is set and so references to Sparkle
+          # wouldn't fail the build (we just remove Sparkle.framework from the built app after the fact).
+          if ${{ matrix.publish == false }}; then
+            # Currently we pass --disable-sparkle flag when publish==false
+            if objdump -t ${MACVIM_BIN} | grep "_SPU\|_SUUpdate"; then
+              echo 'Found references to Sparkle even when using --disable-sparkle'; false
+            fi
+          fi
+
+      - name: Smoketest (publish)
         if: matrix.publish
         run: |
           set -o verbose
@@ -263,12 +283,6 @@ jobs:
           # Check that libsodium is working
           macvim_excmd -c 'set cryptmethod=xchacha20'
 
-          # Make sure there isn't any dynamic linkage to third-party dependencies in the built binary, as we should only use
-          # static linkage to avoid dependency hell. Test that all those dylib's are in /usr/lib which is bundled with macOS and not third-party.
-          if otool -L ${VIM_BIN} | grep '\.dylib\s' | grep -v '^\s*/usr/lib/'; then
-            echo 'Found external dynamic linkage!'; false
-          fi
-
           # Make sure we are building universal x86_64 / arm64 builds and didn't accidentally create a thin app.
           check_arch() {
             local archs=($(lipo -archs "$1"))

src/MacVim/MacVim.xcodeproj/project.pbxproj

@@ -1053,7 +1053,7 @@
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 			shellPath = /bin/sh;
-			shellScript = "./scripts/cleanup-after-build \"$BUILT_PRODUCTS_DIR/$WRAPPER_NAME\"\n";
+			shellScript = "./scripts/cleanup-after-build \"$BUILT_PRODUCTS_DIR/$WRAPPER_NAME\" \"$REMOVE_SPARKLE\"\n";
 			showEnvVarsInLog = 0;
 		};
 		90BD4EF224E0E8B700BF29F2 /* Copy locale message translation files */ = {

src/MacVim/scripts/cleanup-after-build

@@ -3,22 +3,35 @@
 # Utility script to clean up after a MacVim build.
 
 if [[ $# == 0 ]]; then
-    echo "Usage: cleanup-after-build <MacVim_app>"
+    echo "Usage: cleanup-after-build <MacVim_app> <remove_sparkle>"
     exit -1
 fi
 
 set -e
 
 macvim_path=$1
+remove_sparkle=$2
 
-sparkle_xpcservices_symlink="$macvim_path/Contents/Frameworks/Sparkle.framework/XPCServices"
-sparkle_xpcservices="$macvim_path/Contents/Frameworks/Sparkle.framework/Versions/Current/XPCServices"
+if [ "$remove_sparkle" == "1" ]; then
+    sparkle_path="$macvim_path/Contents/Frameworks/Sparkle.framework"
+    if [ -d "$sparkle_path" ]; then
+        # Remove the entire Sparkle folder. Used when --disable-sparkle was set.
+        # Using a clean up script is easier because there isn't an easy way to tell
+        # Xcode not to link/copy it unless we make another target, or dynamically
+        # patch the project file.
+        set -x
+        rm -rf "$sparkle_path"
+    fi
+else
+    sparkle_xpcservices_symlink="$macvim_path/Contents/Frameworks/Sparkle.framework/XPCServices"
+    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"
-    rm "$sparkle_xpcservices_symlink"
+    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"
+        rm "$sparkle_xpcservices_symlink"
+    fi
 fi

src/auto/configure

@@ -5015,7 +5015,7 @@ printf "%s\n" "no" >&6; }
   else
     { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: yes" >&5
 printf "%s\n" "yes" >&6; }
-    XCODEFLAGS="$XCODEFLAGS GCC_PREPROCESSOR_DEFINITIONS='$GCC_PREPROCESSOR_DEFINITIONS DISABLE_SPARKLE=1'"
+    XCODEFLAGS="$XCODEFLAGS GCC_PREPROCESSOR_DEFINITIONS='$GCC_PREPROCESSOR_DEFINITIONS DISABLE_SPARKLE=1' REMOVE_SPARKLE=1"
   fi
 
   if test "$enable_sparkle" == "yes"; then

src/configure.ac

@@ -236,7 +236,7 @@ if test "$vim_cv_uname_output" = Darwin; then
     AC_MSG_RESULT(no)
   else
     AC_MSG_RESULT(yes)
-    XCODEFLAGS="$XCODEFLAGS GCC_PREPROCESSOR_DEFINITIONS='$GCC_PREPROCESSOR_DEFINITIONS DISABLE_SPARKLE=1'"
+    XCODEFLAGS="$XCODEFLAGS GCC_PREPROCESSOR_DEFINITIONS='$GCC_PREPROCESSOR_DEFINITIONS DISABLE_SPARKLE=1' REMOVE_SPARKLE=1"
   fi
 
   if test "$enable_sparkle" == "yes"; then