Merge pull request #1443 from ychin/build-libsodium-gettext-arm64

Build gettext/libsodium universal binary properly in CI

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

@@ -1,5 +1,14 @@
 name: Universal package
 description: Create universal Homebrew package which contains x86_64 and arm64
+
+# Instead of using the default binary installed by Homebrew, we need to build our own because third-party libraries are
+# statically linked in MacVim, and need to be built against MACOSX_DEPLOYMENT_TARGET to ensure the built binary will
+# work on supported macOS versions. Another reason for building our own custom package is to build a unviersal binary
+# 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.
+
 inputs:
   formula:
     description: Formura name
@@ -18,9 +27,18 @@ runs:
         set -o pipefail
         formula=${{ inputs.formula }}
 
-        # Patch the official Homebrew formula to explicitly build for min deployment target
+        # Need to make sure we get the latest before patching. Otherwise Homebrew may later try to get the latest
+        # version and stomp what we have here.
+        brew update
+
+        # 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' >${formula}.rb
+          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
 
         # Uninstall the already installed formula because we want to build our own
         brew uninstall --ignore-dependencies ${formula} || true
@@ -36,7 +54,7 @@ runs:
       uses: actions/cache@v3
       with:
         path: /usr/local/Cellar/${{ inputs.formula }}
-        key: ${{ inputs.formula }}-homebrew-cache-patched-unified-xcode${{ steps.setup-formula.outputs.xcode_version }}-${{ hashFiles(format('{0}.rb', inputs.formula)) }}
+        key: ${{ inputs.formula }}-homebrew-cache-custom-unified-xcode${{ steps.setup-formula.outputs.xcode_version }}-${{ hashFiles(format('{0}.rb', inputs.formula)) }}
 
     - name: Install formula
       shell: bash
@@ -51,38 +69,16 @@ runs:
         brew unlink ${formula} && brew link ${formula}
         echo '::endgroup::'
 
-    - name: Create universal binaries with arm64 bottle
-      if: steps.cache-keg.outputs.cache-hit != 'true'
-      shell: bash
-      run: |
-        echo '::group::Create universal binaries with arm64 bottle'
-        set -o verbose
-        formula=${{ inputs.formula }}
+        echo '::group::Verify built version'
         contents=($(IFS=,; for x in ${{ inputs.contents }}; do echo ${x}; done))
 
-        # Manually download and extract a bottle for arm64
-        source /dev/stdin <<<"$(brew info --json ${formula} | \
-          jq -r '.[0] | "bottle_url=\(.bottle.stable.files.arm64_big_sur.url)", "formula_ver=\(.versions.stable)", "formula_rev=\(.revision)"')"
-        if [[ ${formula_rev} -ne 0 ]]; then
-          formula_ver=${formula_ver}_${formula_rev}
-        fi
-
-        workdir=${formula}_download
-        mkdir ${workdir}
-        cd ${workdir}
-        wget --no-verbose --header 'Authorization: Bearer QQ==' -O ${formula}.tar.gz ${bottle_url}
-        tar xf ${formula}.tar.gz
-
         for content in "${contents[@]}"; do
-          # Just for diagnostics, print out the old archs. This should be a thin binary (x86_64)
-          lipo -info /usr/local/${content}
-
-          # Create a universal binary by patching the custom built x86_64 one with the downloaded arm64 one.
-          # Modify the actual binaries in /usr/local/Cellar instead of the symlinks to allow caching to work.
-          lipo -create -output /usr/local/Cellar/${formula}/${formula_ver}/${content} \
-            /usr/local/Cellar/${formula}/${formula_ver}/${content} ./${formula}/${formula_ver}/${content}
-
-          # Print out the new archs and verify they are universal with 2 archs.
+          # Print out the archs and verify they are universal fat binary.
           lipo -info /usr/local/${content} | grep 'x86_64 arm64'
+
+          # Make sure deployment target is correct. Later macOS versions have a different binary format (just search for
+          # "minos") but for 10.13 we need to look for LC_VERSION_MIN_MACOSX.
+          otool -l /usr/local/${content} | grep -A 2 LC_VERSION_MIN_MACOSX | tail -1 | grep "${MACOSX_DEPLOYMENT_TARGET}"
         done
+
         echo '::endgroup::'

.github/workflows/ci-macvim.yaml

@@ -94,17 +94,6 @@ jobs:
 
       # Set up, install, and cache gettext library for localization.
       #
-      # Instead of using the default binary installed by Homebrew, need to build our own because gettext is statically
-      # linked in MacVim, and need to be built against MACOSX_DEPLOYMENT_TARGET to ensure the built binary will work on
-      # supported macOS versions.
-      #
-      # In addition, to support building a universal MacVim, we need an arm64 version of gettext as well in order to
-      # create a universal gettext binary to link against (Homebrew only distributes thin binaries and therefore this
-      # has to be done manually). To do that, we will just pull the bottle directly from Homebrew and patch it in using
-      # lipo. We can't use normal brew commands to get the bottle because brew doesn't natively support cross-compiling
-      # and we are running CI on x86_64 Macs. We also don't need to worry about the min deployment target fix on arm64
-      # because all Apple Silicon Macs have to run on macOS 11+.
-
       - name: Set up gettext
         if: matrix.publish
         uses: ./.github/actions/universal-package