From cdeaa647aead84ccca557a5a907be750e2b55727 Mon Sep 17 00:00:00 2001 From: Yee Cheng Chin Date: Sun, 11 Jan 2026 00:49:34 -0800 Subject: [PATCH] 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 | 32 +++++++++++++++----- .github/workflows/macvim-buildtest.yaml | 18 +++++++++++ ci/config.mk.brew-libiconv.sed | 8 +++++ src/testdir/test_macvim.vim | 1 + 4 files changed, 51 insertions(+), 8 deletions(-) create mode 100644 ci/config.mk.brew-libiconv.sed diff --git a/.github/actions/universal-package/action.yml b/.github/actions/universal-package/action.yml index 6d5f0a9e5a..0ea6df8532 100644 --- a/.github/actions/universal-package/action.yml +++ b/.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' diff --git a/.github/workflows/macvim-buildtest.yaml b/.github/workflows/macvim-buildtest.yaml index d25e03b670..5bee202306 100644 --- a/.github/workflows/macvim-buildtest.yaml +++ b/.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). diff --git a/ci/config.mk.brew-libiconv.sed b/ci/config.mk.brew-libiconv.sed new file mode 100644 index 0000000000..6bfc3d6919 --- /dev/null +++ b/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/ diff --git a/src/testdir/test_macvim.vim b/src/testdir/test_macvim.vim index 18e13c8254..6feb090b16 100644 --- a/src/testdir/test_macvim.vim +++ b/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"')