Repository navigation
Proper Linux compiler support/documentation - #303
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
| - name: Install Clang 21 | ||
| if: runner.os == 'Linux' | ||
| shell: bash | ||
| run: client-sdk-rust/.github/scripts/install-clang.sh |
There was a problem hiding this comment.
Context: this one upstream downloads a ton more than we need, this was taking a few minutes in CI. The below list is the subset of things we need. I may advocate for moving the upstream script to do this eventually
|
|
||
| ## Prerequisites | ||
|
|
||
| ### Common to all platforms |
There was a problem hiding this comment.
I spent a lot of time by hand reorganizing and adding things here, this diff is rough, probably easiest to just render the markdown and read through the new flow
| discovers libclang from the system paths once `libclang-dev` is installed; if | ||
| not, point `LIBCLANG_PATH` at your LLVM's `lib` directory (e.g. | ||
| `/usr/lib/llvm-18/lib` on Ubuntu 24.04). | ||
| On Linux, install `clang-21` as shown above. Rust bindgen uses libclang to |
There was a problem hiding this comment.
This was an AI change, feels a bit chatty, open to adjusting
There was a problem hiding this comment.
Devin Review found 4 potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
|
|
||
| OS_TYPE="$(detect_os)" | ||
|
|
||
| configure_macos_toolchain() { |
There was a problem hiding this comment.
just want to make sure, is this needed for this linux clang thing?
or some other improvement ?
and it seems configure_macos_toolchain() unconditionally overwrites CC/CXX with the xcrun paths, whereas the Linux path honors pre-set CC/CXX (setup-linux-toolchain.sh:70-79).
There was a problem hiding this comment.
This is mildly related to the Linux work but not directly. We can remove this if you prefer, but the idea was given our docs callout Xcode's installation of Clang, we should do some hardening to find the toolchain in build.sh. The reason this exists is I also had a configure_linux_toolchain() function in this file, but decided to factor it out (into setup-linux-toolchain.sh as you called out) so downstream linux projects could call it out of the SDK submodule.
Happy to adjust direction here/or remove it, thoughts?
|
lgtm, thanks |
stephen-derosa
left a comment
There was a problem hiding this comment.
one request is to also pair up a PR for the examples repo (if needed) to avoid breaking that
|
|
||
| <a id="clang-compiler-usage-on-linux"></a> | ||
|
|
||
| ### Clang compiler usage on Linux |
There was a problem hiding this comment.
curious on the motivation for this being here. I almost think we could put in release notes and move on. Fine keeping it in as a footer too
There was a problem hiding this comment.
Probably me over thinking it. I was pretty shocked to learn we had silently moved compilers under the hood, and esp. from GCC (which I associate with Linux) over to Clang, so I figured it was worth explaining on why if someone's coming into this doc fresh to build on Linux and is also surprised by this.
Totally open to moving to release notes instead, not a bad idea
There was a problem hiding this comment.
your call -- since these docs are dense it might be nice to trim where possible
|
/devin review |
|
Could not match GitHub user alan-george-lk to a Devin account. Please make sure your GitHub account is linked in your Devin settings. |
The C++ SDK silently moved compiler requirements from GCC to Clang 21+ as of release
v1.10.1, and this was never announced, properly documented, and scripts were not updated accordingly. As such, a fresh clone of this SDK on Linux did not build out of the box. This PR addresses this alongside documentation, script support, etc.Notable change overview:
gcc->clang-21build.shnow properly supports this. After users install the deps, this script now just works3.20->3.25CMakePresets.json, just a documentation gap. All supported platforms in CI were already past this versiondocs/building.mdwith relevant changes and proper dependency table