Copilot commented on code in PR #49788: URL: https://github.com/apache/arrow/pull/49788#discussion_r3106367898
########## dev/release/07-flightsqlodbc-upload.sh: ########## @@ -0,0 +1,194 @@ +#!/usr/bin/env bash +# +# 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. +# +# FlightSQL ODBC Release Signing Script +# +# This script handles the signing of FlightSQL ODBC Windows binaries and MSI +# installer. It requires jsign to be configured with ASF code signing +# credentials. Keep reading below: +# +# Required environment variables: +# +# ESIGNER_STOREPASS - The ssl.com credentials in "username|password" format +# ESIGNER_KEYPASS - The ssl.com eSigner secret code (not the PIN) +# +# Set these in .env. +# +# How to get ESIGNER_KEYPASS: +# +# 1. Log into ssl.com +# 2. In your Dashboard, under "invitations", click the link under the order. Or +# go to Orders, find the order, expand the order, and click "certificate +# details" +# 3. Enter your PIN to get your OTP. This is ESIGNER_KEYPASS. +# +# If you don't have access, see https://infra.apache.org/code-signing-use.html. + +set -e +set -u +set -o pipefail + +SOURCE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +if [ "$#" -ne 2 ]; then + echo "Usage: $0 <version> <rc-num>" + exit 1 +fi + +if [ -z "${ESIGNER_STOREPASS:-}" ]; then + echo "ERROR: ESIGNER_STOREPASS is not set" >&2 + exit 1 +fi +if [ -z "${ESIGNER_KEYPASS:-}" ]; then + echo "ERROR: ESIGNER_KEYPASS is not set" >&2 + exit 1 +fi + +. "${SOURCE_DIR}/utils-env.sh" + +version=$1 +rc=$2 + +version_with_rc="${version}-rc${rc}" +tag="apache-arrow-${version_with_rc}" + +dll_unsigned="arrow_flight_sql_odbc_unsigned.dll" +dll_signed="arrow_flight_sql_odbc.dll" + +: "${GITHUB_REPOSITORY:=apache/arrow}" + +: "${PHASE_DEFAULT=1}" +: "${PHASE_SIGN_DLL=${PHASE_DEFAULT}}" +: "${PHASE_BUILD_MSI=${PHASE_DEFAULT}}" +: "${PHASE_SIGN_MSI=${PHASE_DEFAULT}}" Review Comment: The PHASE_* defaults use `${var=default}` which only assigns when the variable is unset (not when it is set but empty). If someone runs with `PHASE_DEFAULT=` (or any PHASE_* set to an empty string), the subsequent numeric `-eq` tests will error. Use the `:=` form (and keep the quoting) to default unset-or-empty values, consistent with other `dev/release` scripts. ```suggestion : "${PHASE_DEFAULT:=1}" : "${PHASE_SIGN_DLL:=${PHASE_DEFAULT}}" : "${PHASE_BUILD_MSI:=${PHASE_DEFAULT}}" : "${PHASE_SIGN_MSI:=${PHASE_DEFAULT}}" ``` ########## dev/release/07-flightsqlodbc-upload.sh: ########## @@ -0,0 +1,194 @@ +#!/usr/bin/env bash +# +# 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. +# +# FlightSQL ODBC Release Signing Script +# +# This script handles the signing of FlightSQL ODBC Windows binaries and MSI +# installer. It requires jsign to be configured with ASF code signing +# credentials. Keep reading below: +# +# Required environment variables: +# +# ESIGNER_STOREPASS - The ssl.com credentials in "username|password" format +# ESIGNER_KEYPASS - The ssl.com eSigner secret code (not the PIN) +# +# Set these in .env. +# +# How to get ESIGNER_KEYPASS: +# +# 1. Log into ssl.com +# 2. In your Dashboard, under "invitations", click the link under the order. Or +# go to Orders, find the order, expand the order, and click "certificate +# details" +# 3. Enter your PIN to get your OTP. This is ESIGNER_KEYPASS. +# +# If you don't have access, see https://infra.apache.org/code-signing-use.html. + +set -e +set -u +set -o pipefail + +SOURCE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +if [ "$#" -ne 2 ]; then + echo "Usage: $0 <version> <rc-num>" + exit 1 +fi + +if [ -z "${ESIGNER_STOREPASS:-}" ]; then + echo "ERROR: ESIGNER_STOREPASS is not set" >&2 + exit 1 +fi +if [ -z "${ESIGNER_KEYPASS:-}" ]; then + echo "ERROR: ESIGNER_KEYPASS is not set" >&2 + exit 1 +fi + +. "${SOURCE_DIR}/utils-env.sh" + +version=$1 +rc=$2 + +version_with_rc="${version}-rc${rc}" +tag="apache-arrow-${version_with_rc}" + +dll_unsigned="arrow_flight_sql_odbc_unsigned.dll" +dll_signed="arrow_flight_sql_odbc.dll" + +: "${GITHUB_REPOSITORY:=apache/arrow}" + +: "${PHASE_DEFAULT=1}" +: "${PHASE_SIGN_DLL=${PHASE_DEFAULT}}" +: "${PHASE_BUILD_MSI=${PHASE_DEFAULT}}" +: "${PHASE_SIGN_MSI=${PHASE_DEFAULT}}" + +if [ "${PHASE_SIGN_DLL}" -eq 0 ] && [ "${PHASE_BUILD_MSI}" -eq 0 ] && [ "${PHASE_SIGN_MSI}" -eq 0 ]; then + echo "No phases specified. Exiting." + exit 1 +fi + +# Utility function to use jsign to check if a file is signed or not +is_signed() { + local file="$1" + local exit_code=0 + jsign extract --format PEM "${file}" > /dev/null 2>&1 || exit_code=$? + # jsign writes a PEM file even though it also prints to stdout. Clean up after + # it. Use -f since so it still runs on unsigned files without error. + rm -f "${file}.sig.pem" + return ${exit_code} +} + +# Use dev/release/tmp for temporary files +tmp_dir="${SOURCE_DIR}/tmp" +if [ -e "${tmp_dir}" ]; then + echo "ERROR: temp dir already exists: ${tmp_dir}. Remove it manually and run again." >&2 + exit 1 +fi + +if [ "${PHASE_SIGN_DLL}" -gt 0 ]; then + echo "[1/9] Downloading ${dll_unsigned} from release..." + gh release download "${tag}" \ + --repo "${GITHUB_REPOSITORY}" \ + --pattern "${dll_unsigned}" \ + --dir "${tmp_dir}" + if is_signed "${tmp_dir}/${dll_unsigned}"; then + echo "ERROR: ${dll_unsigned} is already signed" >&2 + exit 1 + fi + + echo "[2/9] Signing ${dll_unsigned}..." + echo "NOTE: Running jsign. You may be prompted for your OTP PIN..." + jsign --storetype ESIGNER \ + --alias d97c5110-c66a-4c0c-ac0c-1cd6af812ee6 \ + --storepass "${ESIGNER_STOREPASS}" \ + --keypass "${ESIGNER_KEYPASS}" \ + --tsaurl="http://ts.ssl.com"; \ + --tsmode RFC3161 \ + --alg SHA256 \ + "${tmp_dir}/${dll_unsigned}" + mv "${tmp_dir}/${dll_unsigned}" "${tmp_dir}/${dll_signed}" + if ! is_signed "${tmp_dir}/${dll_signed}"; then + echo "ERROR: ${dll_signed} is not signed" >&2 + exit 1 + fi + + echo "[3/9] Uploading signed DLL to GitHub Release..." + gh release upload "${tag}" \ + --repo "${GITHUB_REPOSITORY}" \ + --clobber \ + "${tmp_dir}/${dll_signed}" + + echo "[4/9] Removing unsigned DLL from GitHub Release..." + gh release delete-asset "${tag}" \ + --repo "${GITHUB_REPOSITORY}" \ + --yes \ + "${dll_unsigned}" +fi + +if [ "${PHASE_BUILD_MSI}" -gt 0 ]; then + echo "[5/9] Triggering odbc_release_step in cpp_extra.yml workflow..." + run_url=$(gh workflow run cpp_extra.yml \ + --repo "${GITHUB_REPOSITORY}" \ + --ref "${tag}" \ + --field odbc_release_step=true 2>&1 | grep -oE 'https://[^ ]+' || true) + run_id=${run_url##*/} # Extract the run ID from the URL (the part after the last slash) + if [ -z "${run_id}" ]; then + echo "ERROR: failed to get run ID from workflow trigger" >&2 + exit 1 + fi + echo "Triggered run: ${run_url}" + Review Comment: `PHASE_BUILD_MSI` triggers `gh workflow run cpp_extra.yml ... --field odbc_release_step=true`, but `.github/workflows/cpp_extra.yml` in this branch does not define `workflow_dispatch` inputs (or any `workflow_dispatch` at all). `gh workflow run` will fail in that case, so the “build MSI” phase can’t work as written. Either add an appropriate `workflow_dispatch` trigger+input to the workflow, or change the script to re-run an existing run via `gh run rerun` / use a workflow that actually supports manual dispatch. ```suggestion echo "[5/9] Re-running cpp_extra.yml workflow for tag ${tag}..." tag_sha=$(gh api \ --repo "${GITHUB_REPOSITORY}" \ "repos/{owner}/{repo}/commits/${tag}" \ --jq '.sha') if [ -z "${tag_sha}" ]; then echo "ERROR: failed to resolve commit SHA for tag ${tag}" >&2 exit 1 fi run_id=$(gh run list \ --repo "${GITHUB_REPOSITORY}" \ --workflow cpp_extra.yml \ --json databaseId,headSha \ --limit 100 \ --jq ".[] | select(.headSha == \"${tag_sha}\") | .databaseId" | head -n 1) if [ -z "${run_id}" ]; then echo "ERROR: failed to find an existing cpp_extra.yml run for tag ${tag} (commit ${tag_sha})" >&2 exit 1 fi gh run rerun "${run_id}" --repo "${GITHUB_REPOSITORY}" run_url="https://github.com/${GITHUB_REPOSITORY}/actions/runs/${run_id}"; echo "Re-ran existing workflow run: ${run_url}" ``` ########## docs/source/developers/release.rst: ########## @@ -264,13 +264,24 @@ Build source and binaries and submit them # NOTE: You need to have GitHub CLI installed to run this script. dev/release/06-matlab-upload.sh <version> <rc-number> + # Sign, build the installer for, and sign the installer for the FlightSQL + # ODBC Windows driver + # + # NOTE: This must be run by a PMC member + # Note: You need to have jsign installed and an available credential from + # ASF to sign artifacts. Not all PMC members will have access to code + # signing. + # Note: The script requires setup of ssl.com environment variables. + # Note: Invoking this script costs money. + dev/release/07-flightsqlodbc-upload.sh <version> <rc_number> Review Comment: The placeholder uses `<rc_number>` here, but the rest of the guide (and the scripts’ usage strings) use `<rc-number>`. Using a consistent placeholder avoids confusion/copy-paste errors in the release checklist. ```suggestion dev/release/07-flightsqlodbc-upload.sh <version> <rc-number> ``` ########## docs/source/developers/release.rst: ########## @@ -264,13 +264,24 @@ Build source and binaries and submit them # NOTE: You need to have GitHub CLI installed to run this script. dev/release/06-matlab-upload.sh <version> <rc-number> + # Sign, build the installer for, and sign the installer for the FlightSQL + # ODBC Windows driver + # + # NOTE: This must be run by a PMC member + # Note: You need to have jsign installed and an available credential from + # ASF to sign artifacts. Not all PMC members will have access to code + # signing. + # Note: The script requires setup of ssl.com environment variables. + # Note: Invoking this script costs money. + dev/release/07-flightsqlodbc-upload.sh <version> <rc_number> + # Move the Release Candidate GitHub Release from draft to published state # This will update the artifacts download URL which will be available for the # verification step. - dev/release/07-publish-gh-release.sh <version> <rc-number> + dev/release/08-publish-gh-release.sh <version> <rc-number> # Start verifications for binaries and wheels - dev/release/08-binary-verify.sh <version> <rc-number> + dev/release/09-binary-verify.sh <version> <rc-number> Review Comment: Introducing `dev/release/09-binary-verify.sh` creates a step-number collision with the existing `dev/release/09-vote-email.sh` / `09-vote-email-test.rb` scripts referenced later in this guide. Having multiple “09-*” steps is confusing and makes the release sequence ambiguous; consider renumbering one of the steps (and updating the corresponding filenames + references) to keep the numeric ordering unique. -- 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]
