build(native): Add c-ares installation to Mac setup#27135
Merged
nmahadevuni merged 1 commit intoprestodb:masterfrom Feb 17, 2026
Merged
build(native): Add c-ares installation to Mac setup#27135nmahadevuni merged 1 commit intoprestodb:masterfrom
nmahadevuni merged 1 commit intoprestodb:masterfrom
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds Homebrew-based installation of the c-ares dependency to the macOS native Presto setup flow and wires it into the existing dependency install sequence. Sequence diagram for macOS Presto dependency installation including c_aressequenceDiagram
actor Developer
participant setup_macos_sh
participant install_presto_deps
participant install_presto_deps_from_brew
participant Homebrew
Developer->>setup_macos_sh: execute script
setup_macos_sh->>install_presto_deps: call function
activate install_presto_deps
install_presto_deps->>install_presto_deps_from_brew: run_and_time install_presto_deps_from_brew
activate install_presto_deps_from_brew
install_presto_deps_from_brew->>Homebrew: install_from_brew c-ares
Homebrew-->>install_presto_deps_from_brew: c-ares installed
deactivate install_presto_deps_from_brew
install_presto_deps->>install_presto_deps: run_and_time install_gperf
install_presto_deps->>install_presto_deps: run_and_time install_proxygen
install_presto_deps->>install_presto_deps: run_and_time install_datasketches
deactivate install_presto_deps
install_presto_deps-->>setup_macos_sh: dependencies installed
setup_macos_sh-->>Developer: setup complete
Flow diagram for updated macOS setup with c_ares brew installationflowchart TD
A[Developer runs setup-macos.sh] --> B[Source common Velox macOS setup]
B --> C[Define MACOS_PRESTO_DEPS = c-ares]
C --> D[Call install_presto_deps]
D --> E[run_and_time install_presto_deps_from_brew]
E --> F[Loop over MACOS_PRESTO_DEPS]
F --> G[install_from_brew c-ares]
G --> H[run_and_time install_gperf]
H --> I[run_and_time install_proxygen]
I --> J[run_and_time install_datasketches]
J --> K[macOS native Presto deps ready]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `presto-native-execution/scripts/setup-macos.sh:30-34` </location>
<code_context>
DATASKETCHES_VERSION="5.2.0"
+MACOS_PRESTO_DEPS="c-ares"
+
+function install_presto_deps_from_brew {
+ for pkg in ${MACOS_PRESTO_DEPS}; do
+ install_from_brew "${pkg}"
+ done
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use a local variable for `pkg` to avoid leaking it into the global scope.
You can declare it like this:
```bash
function install_presto_deps_from_brew {
local pkg
for pkg in ${MACOS_PRESTO_DEPS}; do
install_from_brew "${pkg}"
done
}
```
```suggestion
function install_presto_deps_from_brew {
local pkg
for pkg in ${MACOS_PRESTO_DEPS}; do
install_from_brew "${pkg}"
done
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
44b0758 to
4bc5419
Compare
czentgr
reviewed
Feb 16, 2026
Contributor
czentgr
left a comment
There was a problem hiding this comment.
Looks good. Lets add a comment though.
| source "$(dirname "${BASH_SOURCE[0]}")/../velox/scripts/setup-macos.sh" | ||
| GPERF_VERSION="3.1" | ||
| DATASKETCHES_VERSION="5.2.0" | ||
| MACOS_PRESTO_DEPS="c-ares" |
Contributor
There was a problem hiding this comment.
Let's add a small comment somewhere that identifies where this is used (in this case by proxygen). So we keep track of it.
majetideepak
previously approved these changes
Feb 16, 2026
4bc5419 to
e77fe3d
Compare
aditi-pandit
approved these changes
Feb 17, 2026
Contributor
aditi-pandit
left a comment
There was a problem hiding this comment.
Thanks @nmahadevuni
czentgr
approved these changes
Feb 17, 2026
This was referenced Mar 31, 2026
15 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This change adds 'c-ares' package installation to the Mac setup scripts.
Motivation and Context
After recent changes to FB_OS_VERSION for dependencies, proxygen requires c-ares package.
Impact
No impact
Test Plan
Ran setup script successfully on Mac.
Release Notes
Summary by Sourcery
Build: