amoeba commented on code in PR #55321:
URL: https://github.com/apache/arrow/pull/55321#discussion_r4458052869


##########
.github/actions/odbc-mac/action.yml:
##########
@@ -0,0 +1,110 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: ODBC macOS Reusable
+description: Install dependencies and build the Flight SQL ODBC driver on macOS
+inputs:
+  architecture:
+    description: 'Expected runner architecture: AMD64 or ARM64'

Review Comment:
   ```suggestion
       description: 'Architecture to build for (AMD64 or ARM64)'
   ```



##########
.github/actions/odbc-mac/action.yml:
##########
@@ -0,0 +1,110 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: ODBC macOS Reusable
+description: Install dependencies and build the Flight SQL ODBC driver on macOS
+inputs:
+  architecture:
+    description: 'Expected runner architecture: AMD64 or ARM64'
+    required: true
+  generate-installer:
+    description: 'Run cpack to generate the .pkg installer after the build'

Review Comment:
   ```suggestion
       description: 'Whether to run cpack to generate the .pkg installer after 
the build'
   ```



##########
.github/actions/odbc-mac/action.yml:
##########
@@ -0,0 +1,110 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: ODBC macOS Reusable
+description: Install dependencies and build the Flight SQL ODBC driver on macOS
+inputs:
+  architecture:
+    description: 'Expected runner architecture: AMD64 or ARM64'
+    required: true
+  generate-installer:
+    description: 'Run cpack to generate the .pkg installer after the build'
+    required: false
+    default: 'false'
+runs:
+  using: "composite"
+  steps:
+    - name: Check runner architecture
+      shell: bash
+      run: |
+        case "${{ inputs.architecture }}" in
+          AMD64) expected=x86_64 ;;
+          ARM64) expected=arm64 ;;
+          *)
+            echo "Unknown architecture: ${{ inputs.architecture }}"
+            exit 1
+            ;;
+        esac
+        actual="$(uname -m)"
+        if [ "${actual}" != "${expected}" ]; then
+          echo "Runner is ${actual} but ${{ inputs.architecture }} 
(${expected}) was requested"
+          exit 1
+        fi

Review Comment:
   Seems fine to remove.



##########
.github/workflows/package_odbc.yml:
##########
@@ -179,3 +225,253 @@ jobs:
           gh release upload $env:GITHUB_REF_NAME `
             --clobber `
             Apache-Arrow-Flight-SQL-ODBC-*-win64.msi
+
+  # Upload unsigned non-universal DYLIBs
+  odbc-macos-upload-dylib:
+    name: ODBC macOS Upload Unsigned ${{ matrix.architecture }} DYLIB
+    runs-on: macos-${{ matrix.macos-version }}
+    if: github.event_name == 'push'
+    timeout-minutes: 120
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - { architecture: AMD64, macos-version: "26-intel" }
+          - { architecture: ARM64, macos-version: "26" }
+    env:
+      ARROW_BUILD_TESTS: OFF
+      ARROW_BUILD_TYPE: release
+      ARROW_DEPENDENCY_SOURCE: BUNDLED
+      ARROW_DEPENDENCY_USE_SHARED: OFF
+      ARROW_FLIGHT_SQL_ODBC: ON
+      ARROW_FLIGHT_SQL_ODBC_INSTALLER: ON
+      ARROW_HOME: /tmp/local
+      ARROW_MIMALLOC: OFF
+      MACOSX_DEPLOYMENT_TARGET: "14.0"
+    steps:
+      - name: Checkout Arrow
+        uses: actions/checkout@v7
+        with:
+          persist-credentials: false
+          fetch-depth: 0
+          submodules: recursive
+      - name: Build ODBC macOS
+        uses: ./.github/actions/odbc-mac
+        with:
+          architecture: ${{ matrix.architecture }}
+      - name: Copy Unsigned ODBC DYLIB
+        run: |
+          # The unversioned name libarrow_flight_sql_odbc.dylib is a symlink to
+          # libarrow_flight_sql_odbc.<version>.dylib. Copy the real file, 
keeping
+          # its versioned name, so the upload doesn't hit a dangling symlink.
+          mkdir ${{ matrix.architecture }}-dylib
+          cp "$(realpath build/cpp/${{ env.ARROW_BUILD_TYPE 
}}/libarrow_flight_sql_odbc.dylib)" \
+            ${{ matrix.architecture }}-dylib/
+      - name: Upload ODBC DYLIB to the job
+        uses: actions/upload-artifact@v7
+        with:
+          name: flight-sql-odbc-dylib-${{ matrix.architecture }}
+          path: ${{ matrix.architecture 
}}-dylib/libarrow_flight_sql_odbc.*.dylib
+          if-no-files-found: error
+
+  # Upload unsigned universal DYLIB from ARM and AMD DYLIBs
+  odbc-macos-universal-dylib:
+    needs: odbc-macos-upload-dylib
+    name: ODBC macOS Create Unsigned Universal DYLIB
+    runs-on: macos-26
+    timeout-minutes: 30
+    steps:
+      - name: Download ARM64 DYLIB
+        uses: actions/download-artifact@v8
+        with:
+          name: flight-sql-odbc-dylib-ARM64
+          path: arm64-dylib
+      - name: Download AMD64 DYLIB
+        uses: actions/download-artifact@v8
+        with:
+          name: flight-sql-odbc-dylib-AMD64
+          path: amd64-dylib
+      - name: Create universal DYLIB
+        run: |
+          set -euo pipefail
+
+          arm64_dylib="$(ls arm64-dylib/libarrow_flight_sql_odbc.*.dylib)"
+          amd64_dylib="$(ls amd64-dylib/libarrow_flight_sql_odbc.*.dylib)"
+          dylib_name="$(basename "${arm64_dylib}")"
+          if [ "$(basename "${amd64_dylib}")" != "${dylib_name}" ]; then
+            echo "ARM64 and AMD64 DYLIB names differ: ${arm64_dylib} vs 
${amd64_dylib}"
+            exit 1
+          fi
+
+          # Rename unsigned DYLIB 
+          mkdir universal-dylib
+          
unsigned_path="universal-dylib/libarrow_flight_sql_odbc_unsigned.${dylib_name#libarrow_flight_sql_odbc.}"
+          lipo -create "${arm64_dylib}" "${amd64_dylib}" -output 
"${unsigned_path}"
+          unsigned_archs="$(lipo -archs "${unsigned_path}")"
+          echo "${unsigned_path}: ${unsigned_archs}"
+      - name: Upload universal DYLIB to the job
+        uses: actions/upload-artifact@v7
+        with:
+          name: flight-sql-odbc-dylib-universal
+          path: universal-dylib/libarrow_flight_sql_odbc_unsigned.*.dylib
+          if-no-files-found: error
+
+  odbc-dylib-release:
+    needs: odbc-macos-universal-dylib
+    name: Upload Unsigned ODBC DYLIB to Release
+    runs-on: ubuntu-latest
+    permissions:
+      # Upload to GitHub Release
+      contents: write
+    steps:
+      - name: Checkout Arrow
+        uses: actions/checkout@v7
+        with:
+          persist-credentials: false
+          fetch-depth: 0
+          submodules: recursive
+      - name: Download the universal DYLIB
+        uses: actions/download-artifact@v8
+        with:
+          name: flight-sql-odbc-dylib-universal
+      - name: Wait for creating GitHub Release
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          dev/release/utils-watch-gh-workflow.sh \
+            ${GITHUB_REF_NAME} \
+            release_candidate.yml
+      - name: Upload the artifacts to GitHub Release
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          gh release upload ${GITHUB_REF_NAME} \
+            --clobber \
+            libarrow_flight_sql_odbc_unsigned.*.dylib
+
+  # The ARM64 base PKG is not published to the release, so it is rebuilt here
+  # (the release step runs as a separate workflow run, so artifacts from the
+  # tag-triggered run are not available) and the signed universal dylib from
+  # the release is swapped in.
+  odbc-macos-upload-pkg:

Review Comment:
   I think this was the result of my comment in 
https://github.com/apache/arrow/pull/55321#discussion_r4447486220. I think we 
want to avoid rebuilding here. My comment was that I didn't like seeing just 
the unsigned ARM64 .pkg in the release. This is the .pkg we turn into the 
universal .pkg by modifying it. The reason was that it could be confusing to 
see just the ARM64 .pkg in the release and not the AMD64 one but:
   
   1. It's not _that_ confusing
   2. We'll eventually drop AMD64 macOS anyway
   
   During the reviews of the Windows PRs for this, we decided to keep temporary 
artifacts like the unsigned Windows DLL in the release which I think is a good 
idea. So it'd be good to do that here too and keep the ARM64 base .pkg in the 
release.
   
   Maybe we can just not worry about having the unsigned ARM64 base .pkg in the 
release without the accompanying AMD64 .pkg?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to