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


##########
.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:

Review Comment:
   ```suggestion
     odbc-macos-upload-universal-dylib:
   ```



##########
.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:
   why do we have to rebuild at this point? At this point of the release we 
should just use already built artifacts don't we?



##########
.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 slightly defensive :) I personally would just remove this check as I 
don't think the added value is enough, but I am +0 here.



##########
.github/workflows/package_odbc.yml:
##########
@@ -124,10 +124,54 @@ jobs:
             --clobber \
             arrow_flight_sql_odbc_unsigned.dll
 
+  # skip the process if MSI/PKG artifacts with signed 
+  # ODBC libraries are already generated 

Review Comment:
   I am reopening this as I think @amoeba has a point here. I think AI tends to 
be overly defensive on lots of places. In general those defenses are worth 
taking a look at but in this case I think adding more is detrimental for the 
release.
   In this specific case it will work as is but if in the future we make 
changes to other workflows for example changing the tag or the name of the 
assets this is a place where we have to update "stuff". The problem is that we 
might forget to do so and this would require a new RC as it will only fail mid 
release. I'd rather remove this step entirely as if this fails it would still 
fail the following steps nonetheless.
   In my opinion this only adds maintenance burden without adding too much 
value.



-- 
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