kou commented on code in PR #48054: URL: https://github.com/apache/arrow/pull/48054#discussion_r2666805200
########## cpp/src/arrow/flight/sql/odbc/README: ########## Review Comment: Could you add `.md` extension? ########## cpp/src/arrow/flight/sql/odbc/install/windows/arrow-flight-sql-odbc.wxs: ########## @@ -0,0 +1,37 @@ +<?xml version="1.0" encoding="windows-1252"?> + +<!-- + 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. +--> + +<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs";> + <Fragment> + <DirectoryRef Id="TARGETDIR"> + <Component Id="ODBCRegistryEntries" Guid="C348B9F7-5E8D-415D-A001-D92F2D4EA013"> + <RegistryValue Root='HKLM' Key='Software\ODBC\ODBCINST.INI\ODBC Drivers' Name='Apache Arrow Flight SQL ODBC Driver' Type='string' Value='Installed'/> + + <RegistryKey Root="HKLM" + Key="Software\ODBC\ODBCINST.INI\Apache Arrow Flight SQL ODBC Driver"> + <!-- CM_FP_arrow_flight_sql_odbc.bin.arrow_flight_sql_odbc.dll is a generated variable value from build\_CPack_Packages\win64\WIX\files.wxs --> + <RegistryValue Type="string" Name="DriverODBCVer" Value="03.00"/> Review Comment: Is this the version of our Flight SQL ODBC version? Can we use the same version as Apache Arrow C++? ########## .github/workflows/cpp_extra.yml: ########## @@ -433,7 +434,24 @@ jobs: # GH-48269 TODO: Enable Flight & Flight SQL testing in MSVC CI # GH-48547 TODO: enable ODBC tests after GH-48270 and GH-48269 are resolved. - # GH-47787 TODO: Build ODBC installer + - name: Install WiX Toolset + shell: pwsh + run: | + Invoke-WebRequest -Uri https://github.com/wixtoolset/wix/releases/download/v6.0.0/wix-cli-x64.msi -OutFile wix-cli-x64.msi + Start-Process -FilePath wix-cli-x64.msi -ArgumentList '/quiet', 'Include_freethreaded=1' -Wait + echo "C:\Program Files\WiX Toolset v6.0\bin\" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append + - name: Build MSI ODBC installer + shell: pwsh + run: | + # Verify WiX version + wix --version + cd "${{ github.workspace }}\build\cpp" Review Comment: Can we simplify this? ```suggestion cd "build\cpp" ``` ########## cpp/src/arrow/flight/sql/odbc/CMakeLists.txt: ########## @@ -38,10 +36,28 @@ add_subdirectory(tests) arrow_install_all_headers("arrow/flight/sql/odbc") +# ODBC Release information +set(ODBC_PACKAGE_VERSION_MAJOR "1") +set(ODBC_PACKAGE_VERSION_MINOR "0") +set(ODBC_PACKAGE_VERSION_PATCH "0") +set(ODBC_PACKAGE_NAME "Apache Arrow Flight SQL ODBC") +set(ODBC_PACKAGE_VENDOR "Apache Arrow") Review Comment: Could you use "Apache Software Foundation"? ########## cpp/src/arrow/flight/sql/odbc/CMakeLists.txt: ########## @@ -75,3 +91,73 @@ add_arrow_lib(arrow_flight_sql_odbc foreach(LIB_TARGET ${ARROW_FLIGHT_SQL_ODBC_LIBRARIES}) target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_FLIGHT_SQL_ODBC_EXPORTING) endforeach() + +# Construct ODBC Windows installer. Only Release installer is supported +if(ARROW_FLIGHT_SQL_ODBC_INSTALLER) + + include(InstallRequiredSystemLibraries) + + set(CPACK_RESOURCE_FILE_LICENSE + "${CMAKE_CURRENT_SOURCE_DIR}/../../../../../../LICENSE.txt") + # Tentative version 1.0.0 + set(CPACK_PACKAGE_VERSION_MAJOR ${ODBC_PACKAGE_VERSION_MAJOR}) + set(CPACK_PACKAGE_VERSION_MINOR ${ODBC_PACKAGE_VERSION_MINOR}) + set(CPACK_PACKAGE_VERSION_PATCH ${ODBC_PACKAGE_VERSION_PATCH}) + + set(CPACK_PACKAGE_NAME ${ODBC_PACKAGE_NAME}) + set(CPACK_PACKAGE_VENDOR ${ODBC_PACKAGE_VENDOR}) + set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "Apache Arrow Flight SQL ODBC Driver") + set(CPACK_PACKAGE_CONTACT "#GH-47787 TODO arrow maintainers") + + # GH-47876 TODO: set up `arrow_flight_sql_odbc` component for macOS Installer + # GH-47877 TODO: set up `arrow_flight_sql_odbc` component for Linux Installer + if(WIN32) + # Install ODBC and its Arrow dependencies + install(PROGRAMS ${CMAKE_INSTALL_SYSTEM_RUNTIME_LIBS} + DESTINATION bin + COMPONENT arrow_flight_sql_odbc) + install(TARGETS arrow_shared + arrow_compute_shared + arrow_flight_shared + arrow_flight_sql_shared + arrow_flight_sql_odbc_shared + RUNTIME_DEPENDENCIES + PRE_EXCLUDE_REGEXES + "api-ms-.*" + "ext-ms-.*" + POST_EXCLUDE_REGEXES + ".*system32/.*\\.dll" + RUNTIME DESTINATION bin COMPONENT arrow_flight_sql_odbc) + + set(CPACK_WIX_EXTRA_SOURCES + "${CMAKE_CURRENT_SOURCE_DIR}/install/windows/arrow-flight-sql-odbc.wxs") + set(CPACK_WIX_PATCH_FILE + "${CMAKE_CURRENT_SOURCE_DIR}/install/windows/arrow-flight-sql-odbc-patch.xml") + + set(CPACK_WIX_UI_BANNER + "${CMAKE_CURRENT_SOURCE_DIR}/install/windows/arrow-wix-banner.bmp") + endif() + + get_cmake_property(CPACK_COMPONENTS_ALL COMPONENTS) + set(CPACK_COMPONENTS_ALL "arrow_flight_sql_odbc") + + if(WIN32) + # WiX msi installer on Windows + # CPack is compatible with WiX V.5 and V.6 + set(CPACK_GENERATOR "WIX") + set(CPACK_WIX_VERSION 4) + + # Upgrade GUID is required to be unchanged for ODBC installer to upgrade + set(CPACK_WIX_UPGRADE_GUID "DBF27A18-F8BF-423F-9E3A-957414D52C4B") + set(CPACK_WIX_PRODUCT_GUID "279D087B-93B5-4DC3-BA69-BCF485022A26") + endif() + # GH-47876 TODO: create macOS Installer using cpack + # GH-47877 TODO: create Linux Installer using cpack + + # Load CPack after all CPACK* variables are set + include(CPack) + cpack_add_component(arrow_flight_sql_odbc + DISPLAY_NAME "ODBC library" + DESCRIPTION "ODBC library bin, required to install" Review Comment: I'm not sure whether this is used but do we need to include "Apache Arrow Flight SQL" or something in them? ########## cpp/src/arrow/flight/sql/odbc/CMakeLists.txt: ########## @@ -38,10 +36,28 @@ add_subdirectory(tests) arrow_install_all_headers("arrow/flight/sql/odbc") +# ODBC Release information +set(ODBC_PACKAGE_VERSION_MAJOR "1") +set(ODBC_PACKAGE_VERSION_MINOR "0") +set(ODBC_PACKAGE_VERSION_PATCH "0") Review Comment: Can we use the same version as Apache Arrow C++? ########## cpp/src/arrow/flight/sql/odbc/CMakeLists.txt: ########## @@ -75,3 +91,73 @@ add_arrow_lib(arrow_flight_sql_odbc foreach(LIB_TARGET ${ARROW_FLIGHT_SQL_ODBC_LIBRARIES}) target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_FLIGHT_SQL_ODBC_EXPORTING) endforeach() + +# Construct ODBC Windows installer. Only Release installer is supported +if(ARROW_FLIGHT_SQL_ODBC_INSTALLER) + + include(InstallRequiredSystemLibraries) + + set(CPACK_RESOURCE_FILE_LICENSE + "${CMAKE_CURRENT_SOURCE_DIR}/../../../../../../LICENSE.txt") + # Tentative version 1.0.0 + set(CPACK_PACKAGE_VERSION_MAJOR ${ODBC_PACKAGE_VERSION_MAJOR}) + set(CPACK_PACKAGE_VERSION_MINOR ${ODBC_PACKAGE_VERSION_MINOR}) + set(CPACK_PACKAGE_VERSION_PATCH ${ODBC_PACKAGE_VERSION_PATCH}) + + set(CPACK_PACKAGE_NAME ${ODBC_PACKAGE_NAME}) + set(CPACK_PACKAGE_VENDOR ${ODBC_PACKAGE_VENDOR}) + set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "Apache Arrow Flight SQL ODBC Driver") + set(CPACK_PACKAGE_CONTACT "#GH-47787 TODO arrow maintainers") Review Comment: Do we need an e-mail address here? If so, could you use `[email protected]`? ########## .github/workflows/cpp_extra.yml: ########## @@ -433,7 +434,24 @@ jobs: # GH-48269 TODO: Enable Flight & Flight SQL testing in MSVC CI # GH-48547 TODO: enable ODBC tests after GH-48270 and GH-48269 are resolved. - # GH-47787 TODO: Build ODBC installer + - name: Install WiX Toolset + shell: pwsh + run: | + Invoke-WebRequest -Uri https://github.com/wixtoolset/wix/releases/download/v6.0.0/wix-cli-x64.msi -OutFile wix-cli-x64.msi + Start-Process -FilePath wix-cli-x64.msi -ArgumentList '/quiet', 'Include_freethreaded=1' -Wait + echo "C:\Program Files\WiX Toolset v6.0\bin\" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append + - name: Build MSI ODBC installer + shell: pwsh + run: | + # Verify WiX version + wix --version + cd "${{ github.workspace }}\build\cpp" + cpack + - name: Upload the artifacts to the job + uses: actions/upload-artifact@v6 + with: + name: flight-sql-odbc-msi-installer + path: ${{ github.workspace }}\build\cpp\Apache Arrow Flight SQL ODBC-1.0.0-win64.msi Review Comment: Can we simplify this? ```suggestion path: build\cpp\Apache Arrow Flight SQL ODBC-1.0.0-win64.msi ``` ########## cpp/src/arrow/flight/sql/odbc/README: ########## @@ -0,0 +1,63 @@ +<!--- + 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. +--> + +## Steps to Register the 64-bit Apache Arrow ODBC driver on Windows + +After the build succeeds, the ODBC DLL will be located in +`build\debug\Debug` for a debug build and `build\release\Release` for a release build. + +1. Open Windows Power Shell as administrator. + +2. Register your ODBC DLL: + Need to replace <path\to\repo> with actual path to repository in the commands. + + i. `cd to repo.` + ii. `cd <path\to\repo>` + iii. Run script to register your ODBC DLL as Apache Arrow Flight SQL ODBC Driver + `.\cpp\src\arrow\flight\sql\odbc\tests\install_odbc.cmd <path\to\repo>\cpp\build\< release | debug >\< Release | Debug>\arrow_flight_sql_odbc.dll` + Example command for reference: + `.\cpp\src\arrow\flight\sql\odbc\tests\install_odbc.cmd C:\path\to\arrow\cpp\build\release\Release\arrow_flight_sql_odbc.dll` Review Comment: ```suggestion Need to replace <path\to\repo> with actual path to repository in the commands. 1. `cd to repo.` 2. `cd <path\to\repo>` 3. Run script to register your ODBC DLL as Apache Arrow Flight SQL ODBC Driver ``` .\cpp\src\arrow\flight\sql\odbc\tests\install_odbc.cmd <path\to\repo>\cpp\build\< release | debug >\< Release | Debug>\arrow_flight_sql_odbc.dll ``` Example command for reference: ``` .\cpp\src\arrow\flight\sql\odbc\tests\install_odbc.cmd C:\path\to\arrow\cpp\build\release\Release\arrow_flight_sql_odbc.dll ``` ``` ########## cpp/src/arrow/flight/sql/odbc/install/windows/versioninfo.rc.in: ########## @@ -0,0 +1,54 @@ +// 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. + +#define VER_FILEVERSION @VER_FILEVERSION@ +#define VER_FILEVERSION_STR "@VER_FILEVERSION_STR@\0" Review Comment: Is `\0` needed? ########## cpp/src/arrow/flight/sql/odbc/install/windows/arrow-flight-sql-odbc.wxs: ########## @@ -0,0 +1,37 @@ +<?xml version="1.0" encoding="windows-1252"?> Review Comment: Is `windows-1252` expected? Can we use `UTF-8` instead? -- 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]
