macvim-dev / macvim

Vim - the text editor - for macOS
https://macvim.org
Vim License
7.47k stars 680 forks source link

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

Closed ychin closed 1 year ago

ychin commented 1 year ago

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.