GH-49537: [C++][FlightRPC] Windows CI to Support ODBC DLL & MSI Signing#49603
GH-49537: [C++][FlightRPC] Windows CI to Support ODBC DLL & MSI Signing#49603alinaliBQ wants to merge 5 commits intoapache:mainfrom
Conversation
* Add draft code for CI A and CI B Attempt workflow dispatch Only ODBC Windows original workflow should run. Later need to add `github.event_name != 'workflow_dispatch' ||` to all existing workflows after uncomment Use `GITHUB_REF_NAME` directly via push Add `workflow_dispatch` definitions Add `ODBC Windows Upload DLL` Use common ODBC Windows environment variables Use ODBC as composite action Create cpp_odbc.yml Initial draft temp disable test step Temp disable non-ODBC Windows workflows * Clean Up Code * Remove comments * Fix Installer path for MSI
| - name: Upload the artifacts to GitHub Release | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| gh release upload ${GITHUB_REF_NAME} \ | ||
| --clobber \ | ||
| arrow_flight_sql_odbc_unsigned.dll |
There was a problem hiding this comment.
Since both DLL and MSI need to be signed and unsigned DLL is harder to catch, uploading as arrow_flight_sql_odbc_unsigned.dll to make it clear on GitHub release if the DLL is unsigned.
|
I did an empty commit (a522393) and got this error: The same implementation worked yesterday (see https://github.com/apache/arrow/actions/runs/23622018872/job/68803175375). Seems that GitHub might have updated runner, I will look into this. |
This issue should be resolved now by commit 95bc75b |
| gh release download $env:GITHUB_REF_NAME ` | ||
| --pattern arrow_flight_sql_odbc.dll ` | ||
| --clobber | ||
| - name: Build ODBC Windows |
There was a problem hiding this comment.
Did you have any luck figuring out how to extract just the minimum set of files cpack needs to build the MSI? I see this job is rebuilding everything from scratch so we're building the ODBC driver twice. If it's not possible that's fine, and if we want to work on removing the extra full-build in a follow-up that's fine.
There was a problem hiding this comment.
I am assuming we are referring to the comment here (#49404 (comment)). Yup the approach builds everything twice by design. Unfortunately getting specific files for a CPack build isn't doable as CPack only packages from a single build directory.
This workflow looks for cache, if a cache is hit, the build will be faster. I think this is the cleanest approach. And thanks for your understanding.
| remote_key: ${{ secrets.NIGHTLIES_RSYNC_KEY }} | ||
| remote_host_key: ${{ secrets.NIGHTLIES_RSYNC_HOST_KEY }} | ||
|
|
||
| odbc-release: |
There was a problem hiding this comment.
Could we automatically build and upload the unsigned DLL when we tag a release? I think it could speed things up for the release managers. It looks like the way you have it here, the release manager has to manually run odbc-msvc-upload-dll. Is that just because of the renaming step?
There was a problem hiding this comment.
We do it on the Linux packaging ones. We trigger the job on tag creation:
arrow/.github/workflows/package_linux.yml
Lines 38 to 39 in 9273467
We wait until the Release is created on GitHub so we can use the tar.gz from the release:
arrow/.github/workflows/package_linux.yml
Lines 228 to 235 in 9273467
And we only upload to the release:
arrow/.github/workflows/package_linux.yml
Lines 288 to 295 in 9273467
Some of those steps are only done in the case of tag trigger (so only on RCs).
I think we should use a similar pattern.
There was a problem hiding this comment.
Yes I have changed odbc-msvc-upload-dll to be automatically triggered by the release tag. odbc-msvc-upload-dll will no longer be trigged via workflow dispatch.
From the discussion at #49404, I originally thought we wanted the odbc-msvc-upload-dll workflow to be triggered manually as well.
.github/workflows/cpp_extra.yml
Outdated
| uses: ./.github/actions/odbc-windows | ||
| with: | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| - name: Name Unsigned ODBC DLL |
There was a problem hiding this comment.
| - name: Name Unsigned ODBC DLL | |
| - name: Rename Unsigned ODBC DLL |
.github/workflows/cpp_extra.yml
Outdated
|
|
||
| odbc-msvc-upload-msi: | ||
| needs: check-labels | ||
| name: ODBC Windows Upload Unsigned MSI |
There was a problem hiding this comment.
| name: ODBC Windows Upload Unsigned MSI | |
| name: ODBC Windows Build & Upload Unsigned MSI |
.github/workflows/cpp_extra.yml
Outdated
| 0 0 * * * | ||
| workflow_dispatch: | ||
| inputs: | ||
| odbc_upload: |
There was a problem hiding this comment.
Just a suggestion, this seems clearer as to the intent:
| odbc_upload: | |
| odbc_release_step: |
There was a problem hiding this comment.
yup sure, changed the name to odbc_release_step. I have changed the input to be boolean as well since odbc_release_step will only be used for triggering the MSI step.
.github/workflows/cpp_extra.yml
Outdated
| workflow_dispatch: | ||
| inputs: | ||
| odbc_upload: | ||
| description: 'ODBC Component Upload' |
There was a problem hiding this comment.
| description: 'ODBC Component Upload' | |
| description: 'Which ODBC release step to run' |
There was a problem hiding this comment.
Changed to ODBC MSI release step
.github/workflows/cpp_extra.yml
Outdated
| needs: check-labels | ||
| name: ODBC Windows Upload Unsigned DLL | ||
| runs-on: windows-2022 | ||
| if: inputs.odbc_upload == 'dll' |
There was a problem hiding this comment.
| if: inputs.odbc_upload == 'dll' | |
| if: inputs.odbc_release_step == 'dll' |
.github/workflows/cpp_extra.yml
Outdated
| needs: check-labels | ||
| name: ODBC Windows Upload Unsigned MSI | ||
| runs-on: windows-2022 | ||
| if: inputs.odbc_upload == 'msi' |
There was a problem hiding this comment.
| if: inputs.odbc_upload == 'msi' | |
| if: inputs.odbc_release_step == 'msi' |
Change `odbc-msvc-upload-dll` to be triggered via rc tag and can be invoked manually
alinaliBQ
left a comment
There was a problem hiding this comment.
Addressed comments. Please have another look, thanks!
| gh release download $env:GITHUB_REF_NAME ` | ||
| --pattern arrow_flight_sql_odbc.dll ` | ||
| --clobber | ||
| - name: Build ODBC Windows |
There was a problem hiding this comment.
I am assuming we are referring to the comment here (#49404 (comment)). Yup the approach builds everything twice by design. Unfortunately getting specific files for a CPack build isn't doable as CPack only packages from a single build directory.
This workflow looks for cache, if a cache is hit, the build will be faster. I think this is the cleanest approach. And thanks for your understanding.
| remote_key: ${{ secrets.NIGHTLIES_RSYNC_KEY }} | ||
| remote_host_key: ${{ secrets.NIGHTLIES_RSYNC_HOST_KEY }} | ||
|
|
||
| odbc-release: |
There was a problem hiding this comment.
Yes I have changed odbc-msvc-upload-dll to be automatically triggered by the release tag. odbc-msvc-upload-dll will no longer be trigged via workflow dispatch.
From the discussion at #49404, I originally thought we wanted the odbc-msvc-upload-dll workflow to be triggered manually as well.
.github/workflows/cpp_extra.yml
Outdated
| uses: ./.github/actions/odbc-windows | ||
| with: | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| - name: Name Unsigned ODBC DLL |
.github/workflows/cpp_extra.yml
Outdated
|
|
||
| odbc-msvc-upload-msi: | ||
| needs: check-labels | ||
| name: ODBC Windows Upload Unsigned MSI |
.github/workflows/cpp_extra.yml
Outdated
| 0 0 * * * | ||
| workflow_dispatch: | ||
| inputs: | ||
| odbc_upload: |
There was a problem hiding this comment.
yup sure, changed the name to odbc_release_step. I have changed the input to be boolean as well since odbc_release_step will only be used for triggering the MSI step.
.github/workflows/cpp_extra.yml
Outdated
| workflow_dispatch: | ||
| inputs: | ||
| odbc_upload: | ||
| description: 'ODBC Component Upload' |
There was a problem hiding this comment.
Changed to ODBC MSI release step
.github/workflows/cpp_extra.yml
Outdated
| needs: check-labels | ||
| name: ODBC Windows Upload Unsigned MSI | ||
| runs-on: windows-2022 | ||
| if: inputs.odbc_upload == 'msi' |
.github/workflows/cpp_extra.yml
Outdated
| needs: check-labels | ||
| name: ODBC Windows Upload Unsigned DLL | ||
| runs-on: windows-2022 | ||
| if: inputs.odbc_upload == 'dll' |
Rationale for this change
GH-49537
What changes are included in this PR?
odbc-msvc-upload-dllUpload unsigned DLLodbc-msvc-upload-msiDownload signed DLL and upload unsigned MSIodbc-releaseCI that is replaced by the new CIs.Example of
07-flightsqlodbc-upload.shscript (not tested):We need to either 1) implement a way to get
RUN_IDand then callgh run watch,or 2) enter each command manually and wait for the CI to finish.
Are these changes tested?
Are there any user-facing changes?
N/A