THRIFT-5242: Add Homebrew installation instructions for macOS - #3826
slachiewicz wants to merge 1 commit into
Conversation
82a1e28 to
362d954
Compare
Code reviewFound 2 issues:
Lines 23 to 25 in 362d954 thrift/.github/workflows/build.yml Lines 131 to 133 in 362d954
Lines 60 to 74 in 362d954 Two suggestions, below the bar for the list above but verified:
Lines 28 to 37 in 362d954
Lines 41 to 44 in 362d954 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
362d954 to
dd62e9c
Compare
|
Rewrote the branch after the review above:
Verified: both recipes run as written on macOS 15 (Apple Silicon) → This comment was created with AI assistance. |
Client: doc
Update doc/install/os_x.md to document installing the Thrift compiler
via Homebrew ('brew install thrift') and building from source using
Homebrew dependencies with CMake and Autotools.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
dd62e9c to
640e71f
Compare
|
A limit on the verification above, since I had no clean macOS environment for it:
The macOS CI job covers the Autotools path on a fresh runner; the CMake path has no CI coverage. This comment was created with AI assistance. |
JIRA: THRIFT-5242
Client: doc
doc/install/os_x.mdnow covers the pre-built compiler first,brew install thrift, and then building the compiler from source with Homebrew-provided tools.automake libtool pkg-config cmake bison flex, the set the macOS CI job uses, so./bootstrap.shfinds libtoolize andconfigure.acfinds the pkg-config macros.--disable-libs --disable-testsfor Autotools and-DBUILD_LIBRARIES=OFF -DBUILD_TESTING=OFF -DBUILD_TUTORIALS=OFFfor CMake. Neither needs Boost, libevent or OpenSSL; one sentence tells readers who also want the C++ library what to add.cmake-build, sincecmake -B buildwould reuse the repository's ownbuild/directory, which holds the CMake modules.bisonshipped with macOS is 2.3 and cannot process the grammar, and that Homebrew's is keg-only, so each recipe points at it explicitly.Verified: both recipes run as written on macOS 15 (Apple Silicon) →
thrift -versionprints 0.25.0 from each build. Limits of that check: thesudo … installsteps ran into a scratch prefix, so the/usr/localpermissions were not exercised;flexon the test machine came from Xcode rather than Homebrew; and the unchanged section on building Boost and libevent by hand was not run. The macOS CI job covers the Autotools path on a fresh runner; the CMake path has no CI coverage.This change was created with AI assistance.