Build MacVim binary release with GNU iconv instead of Apple iconv

The builtin iconv in macOS has been quite buggy since macOS 14, when
Apple replaced GNU iconv with a bespoke version. It introduced
backwards-incompatible changes, and behaves oddly in certain character
substitutions.

As such, build the official binary release using GNU iconv instead of
system iconv. This means we have to compile/cache it manually in our CI
just like gettext/libsodium in order to have a universal x86/arm64
binary with the correct deployment target set. We also need to modify
gettext to be built against GNU iconv as well to avoid link-time errors.

Note that this does not affect the Homebrew release of MacVim. The
standard Homebrew gettext is still linked against system iconv, and as
such we can't make an unilateral change without modifying Homebrew's
gettext as well.

This will result in the Vim binary being larger by 2 MB. It's not ideal
but tolerable. If Apple fixes their implementation of iconv we could
revert this in the future.

Related: macvim-dev/macvim#1624

.github/actions/universal-package/action.yml

@@ -7,15 +7,19 @@ description: Create universal Homebrew package which contains x86_64 and arm64
 # that has both x86_64 and arm64 arch, as Homebrew's distributed bottles are thin binaries with only one arch.
 #
 # We still use Homebrew to manage the library because their formulas are up to date and have correct build instructions
-# that will work. This way we don't have to manually configuring and building and updating the package info.
+# that will work. This way we don't have to manually configure, build, and update the package info.
 
 inputs:
   formula:
-    description: Formura name
+    description: Formula name
     required: true
   contents:
     description: Path for contents in package's keg
     required: true
+  gnuiconv:
+    description: Use the Homebrew GNU libiconv instead of system one
+    type: boolean
+    required: false
 runs:
   using: 'composite'
   steps:
@@ -31,14 +35,24 @@ runs:
         # version and stomp what we have here.
         brew update
 
+        brew cat ${formula} >${formula}.rb
+
+        if [[ "${{ inputs.gnuiconv }}" == "true" ]]; then
+          # Modify formula to build using Homebrew libiconv. Usually just adding "depends_on" is enough, but since we
+          # override "CC" to use vanilla system clang, we need to manually inject the compilation/link flags to specify
+          # the locations.
+          sed -i.bak '/^[[:blank:]]*def install$/i\'$'\n  depends_on "libiconv"\n' ${formula}.rb
+          sed -i.bak '/^[[:blank:]]*def install$/a\'$'\n    ENV["CFLAGS"] += " -I'$(brew --prefix)$'/opt/libiconv/include"\n' ${formula}.rb
+          sed -i.bak '/^[[:blank:]]*def install$/a\'$'\n    ENV["LDFLAGS"] += " -L'$(brew --prefix)$'/opt/libiconv/lib"\n' ${formula}.rb
+        fi
+
         # Patch the official Homebrew formula to explicitly build for min deployment target and a universal binary. We
         # also need to explicitly use system Clang because Homebrew's bundled clang script tries to inject -march
         # compiler flags that will cause universal builds to fail as Clang does not like that.
-        brew cat ${formula} | \
-          sed '/^[[:blank:]]*def install$/a\'$'\n    ENV["MACOSX_DEPLOYMENT_TARGET"] = "'${MACOSX_DEPLOYMENT_TARGET}$'"\n' | \
-          sed '/^[[:blank:]]*def install$/a\'$'\n    ENV["CC"] = "/usr/bin/clang"\n' | \
-          sed '/^[[:blank:]]*def install$/a\'$'\n    ENV["CFLAGS"] = "-arch x86_64 -arch arm64"\n' | \
-          sed '/^[[:blank:]]*def install$/a\'$'\n    ENV["LDFLAGS"] = "-arch x86_64 -arch arm64"\n' >${formula}.rb
+        sed -i.bak '/^[[:blank:]]*def install$/a\'$'\n    ENV["MACOSX_DEPLOYMENT_TARGET"] = "'${MACOSX_DEPLOYMENT_TARGET}$'"\n' ${formula}.rb
+        sed -i.bak '/^[[:blank:]]*def install$/a\'$'\n    ENV["CC"] = "/usr/bin/clang"\n' ${formula}.rb
+        sed -i.bak '/^[[:blank:]]*def install$/a\'$'\n    ENV["CFLAGS"] = "-arch x86_64 -arch arm64"\n' ${formula}.rb
+        sed -i.bak '/^[[:blank:]]*def install$/a\'$'\n    ENV["LDFLAGS"] = "-arch x86_64 -arch arm64"\n' ${formula}.rb
 
         # Homebrew requires formula files to be placed in taps and disallows
         # installing from raw paths, so we manually create a taps folder for a
@@ -85,7 +99,9 @@ runs:
         brew list ${formula} &>/dev/null || brew install --quiet --formula -s macvim-dev/deps/${formula}
 
         # If formula was cached, this step is necessary to relink it to brew prefix (e.g. /usr/local)
-        brew unlink ${formula} && brew link ${formula}
+        # The "-f" is there to force link keg-only formulas. Homebrew doesn't provide a command to just link in the
+        # optional /opt/homebrew/opt/... folders, so we have to resort to doing this.
+        brew unlink ${formula} && brew link -f ${formula}
         echo '::endgroup::'
 
         echo '::group::Verify built version'

.github/workflows/macvim-buildtest.yaml

@@ -89,6 +89,15 @@ jobs:
           xcode-select -p
           xcodebuild -version
 
+      # Set up, install, and cache GNU libiconv library to work around Apple iconv issues.
+
+      - name: Set up libiconv
+        if: inputs.publish
+        uses: ./.github/actions/universal-package
+        with:
+          formula: libiconv
+          contents: opt/libiconv/lib/libiconv.a,opt/libiconv/lib/libiconv.dylib
+
       # Set up, install, and cache gettext library for localization.
 
       - name: Set up gettext
@@ -97,6 +106,7 @@ jobs:
         with:
           formula: gettext
           contents: lib/libintl.a,lib/libintl.dylib
+          gnuiconv: true  # gettext needs to match MacVim in using the same version of iconv
 
       # Set up, install, and cache libsodium library for encryption.
 
@@ -222,6 +232,9 @@ jobs:
             sed -i.bak -f ci/config.mk.optimized.sed src/auto/config.mk
           fi
 
+          # Use Homebrew GNU libiconv since Apple iconv has been broken since macOS 14
+          sed -i.bak -f ci/config.mk.brew-libiconv.sed src/auto/config.mk
+
       - name: Modify configure result
         if: inputs.publish
         run: |
@@ -271,6 +284,11 @@ jobs:
             echo 'Found external dynamic linkage!'; false
           fi
 
+          # Make sure we are not using system iconv, which has been buggy since macOS 14.
+          if otool -L ${VIM_BIN} | grep '^\s*/usr/lib/libiconv'; then
+            echo 'Using system iconv! We should be linking against GNU iconv instead.'; 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).

ci/config.mk.brew-libiconv.sed

@@ -0,0 +1,8 @@
+# Use Homebrew GNU libiconv to work around broken Apple iconv. Use static
+# linking as we don't want our binary releases to pull in third-party
+# dependencies.
+#
+# If gettext is configured in the build, it also needs to be built against GNU
+# libiconv. Otherwise we would get a link error from this.
+/^CFLAGS[[:blank:]]*=/s/$/ -I\/opt\/homebrew\/opt\/libiconv\/include/
+/^LIBS[[:blank:]]*=/s/-liconv/\/opt\/homebrew\/opt\/libiconv\/lib\/libiconv.a/

src/testdir/test_macvim.vim

@@ -35,6 +35,7 @@ func Test_macvim_options_commands_exist()
     call assert_true(has('clientserver'), 'Missing feature "clientserver"')
     call assert_true(has('clipboard'), 'Missing feature "clipboard"')
     call assert_true(has('clipboard_working'), 'Missing feature "clipboard_working"')
+    call assert_true(has('iconv'), 'Missing feature "iconv"')
     call assert_true(has('sound'), 'Missing feature "sound"')
     call assert_true(has('terminal'), 'Missing feature "terminal"')
     call assert_true(has('xim'), 'Missing feature "xim"')