Skip to content

GH-48576: [C++][FlightRPC] ODBC: add Mac setup script #48578

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
alinaliBQ wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

GH-48576: [C++][FlightRPC] ODBC: add Mac setup script #48578

alinaliBQ wants to merge 3 commits into from
+163 −0

Conversation

@alinaliBQ
Copy link
Collaborator

@alinaliBQ alinaliBQ commented Dec 17, 2025 •
edited by github-actions bot
Loading

Rationale for this change

#48576

What changes are included in this PR?

  • Added Mac Setup ODBC ini Script

Are these changes tested?

Will be tested in CI when PR is ready for review.
Tested locally on macOS.

Are there any user-facing changes?

N/A

@github-actions
Copy link

github-actions bot commented Dec 17, 2025

⚠️ GitHub issue #48576 has been automatically assigned in GitHub to PR creator.

@alinaliBQ
Copy link
Collaborator Author

alinaliBQ commented Dec 17, 2025

@lidavidm This draft PR for macOS ODBC registration set up script is ready for review. It is pending on #48577 to undraft.

@lidavidm lidavidm changed the title GH-48576: [C++][FlightRPC] Mac Setup ODBC ini Script GH-48576: [C++][FlightRPC] ODBC: add Mac setup script Jan 5, 2026
lidavidm
lidavidm reviewed Jan 5, 2026
View reviewed changes
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to guard the scripts with (e.g.) set -euo pipefail?

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Jan 5, 2026
lidavidm
lidavidm reviewed Jan 5, 2026
View reviewed changes
cpp/src/arrow/flight/sql/odbc/install/mac/install_odbc_ini.sh Outdated
odbc_install_script="${source_dir}/install_odbc.sh"

chmod +x "$odbc_install_script"
. "$odbc_install_script" /Library/Apache/ArrowFlightSQLODBC/lib/libarrow_flight_sql_odbc.dylib
Copy link
Member

@lidavidm lidavidm Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to source the script?

Copy link
Collaborator Author

@alinaliBQ alinaliBQ Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching it, I think it is not necessary. Removed sourcing of the script.

vic-tsang and others added 2 commits January 6, 2026 15:13
@vic-tsang @alinaliBQ
Mac Setup ODBC ini Script
afda216 
* added setup script for mac and platform folders

Co-Authored-By: Alina (Xi) Li <alina.li@improving.com>
Co-Authored-By: vic-tsang <victor.tsang@improving.com>

Update install_odbc.sh
@alinaliBQ
Apply suggestions from David
c64342b 
- Moved admin check upwards
- removed sourcing of script that isn't required
@alinaliBQ alinaliBQ force-pushed the gh-48576-macos-ini-script branch from 3292db5 to c64342b Compare January 6, 2026 23:14
alinaliBQ
alinaliBQ commented Jan 6, 2026
View reviewed changes
Copy link
Collaborator Author

@alinaliBQ alinaliBQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review David, addressed all comments

cpp/src/arrow/flight/sql/odbc/install/mac/install_odbc_ini.sh Outdated
odbc_install_script="${source_dir}/install_odbc.sh"

chmod +x "$odbc_install_script"
. "$odbc_install_script" /Library/Apache/ArrowFlightSQLODBC/lib/libarrow_flight_sql_odbc.dylib
Copy link
Collaborator Author

@alinaliBQ alinaliBQ Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching it, I think it is not necessary. Removed sourcing of the script.

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jan 6, 2026
@alinaliBQ
Copy link
Collaborator Author

alinaliBQ commented Jan 6, 2026

Is it possible to guard the scripts with (e.g.) set -euo pipefail?

@lidavidm Yes, I have added the guards at the beginning of scripts.

lidavidm
lidavidm reviewed Jan 7, 2026
View reviewed changes
cpp/src/arrow/flight/sql/odbc/install/mac/install_odbc.sh Outdated
@@ -0,0 +1,78 @@
#!/bin/sh
Copy link
Member

@lidavidm lidavidm Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we using plain sh, or assuming something else (zsh on macOS?)

Copy link
Collaborator Author

@alinaliBQ alinaliBQ Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch and I have changed to use #!/bin/bash. The scripts assume bash shells on macOS are used.

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Jan 7, 2026
@alinaliBQ
Use #!/bin/bash for scripts
3d97d65 
The scripts assume bash shells on macOS are used.
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jan 7, 2026
@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lidavidm lidavidm lidavidm approved these changes

Assignees

No one assigned

Labels

awaiting merge Awaiting merge Component: C++ Component: FlightRPC

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alinaliBQ @lidavidm @vic-tsang