Install protoc and libclang in Windows bootstrap#11536
Install protoc and libclang in Windows bootstrap#11536SATYAM-PRATIBHAN wants to merge 3 commits into
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @SATYAM-PRATIBHAN on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
This PR is not linked to an issue that is marked with Issue-state enforcement details:
To continue, link this PR to a same-repo issue such as Powered by Oz |
github-actions
Bot
added
the
external-contributor
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is not linked to an issue that is marked with ready-to-implement.
Issue-state enforcement details:
-
Associated same-repo issues checked: none
-
Required readiness label:
ready-to-implement
To continue, link this PR to a same-repo issue such as Closes #123 in the PR description, and make sure that issue has ready-to-implement.
Powered by Oz
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR updates the Windows bootstrap script to advertise and install protoc and LLVM so fresh Windows builds have the tools needed by prost-build and bindgen.
Concerns
- The LLVM install step does not make
libclangdiscoverable to the current bootstrap environment, sobindgencan still fail after the script completes unless the script updatesPATHorLIBCLANG_PATH. - The protobuf install should use the exact WinGet package ID to avoid query ambiguity and match the other automated installs.
Verdict
Found: 0 critical, 1 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| winget install -e --id JRSoftware.InnoSetup | ||
|
|
||
| # protoc (Protocol Buffers compiler) is required by prost-build. | ||
| winget install protobuf |
There was a problem hiding this comment.
💡 [SUGGESTION] Use the exact WinGet package ID so the bootstrap stays non-interactive and consistent with the other package installs.
| winget install protobuf | |
| winget install -e --id Google.Protobuf |
| winget install protobuf | ||
|
|
||
| # LLVM provides libclang, which is required by bindgen. | ||
| winget install -e --id LLVM.LLVM |
There was a problem hiding this comment.
LLVM.LLVM installs LLVM, but the silent WinGet install does not guarantee C:\Program Files\LLVM\bin is available to this PowerShell session, so bindgen can still fail to load libclang after bootstrap. Append the LLVM bin directory to $env:PATH or set LIBCLANG_PATH after installation.
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR updates the Windows bootstrap script to install protoc via Google.Protobuf and LLVM/libclang via LLVM.LLVM, and updates the bootstrap preview text.
Concerns
- The Protobuf PATH fallback points at a hardcoded
%ProgramFiles%\protobuf\binlocation that does not match where winget installs theGoogle.Protobufportable package, so a fresh bootstrap can still leaveprotocunavailable in the current shell.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| # protoc (Protocol Buffers compiler) is required by prost-build. | ||
| winget install -e --id Google.Protobuf | ||
| if (-not (Get-Command -Name protoc -Type Application -ErrorAction SilentlyContinue)) { | ||
| $env:PATH += ";$env:ProgramFiles\protobuf\bin" |
There was a problem hiding this comment.
Google.Protobuf is installed by winget as a portable package under WinGet's package/link directories, not $env:ProgramFiles\protobuf\bin; this fallback can still leave protoc unavailable in the current shell, so resolve and append the actual package bin path.
|
/oz-review |
1 participant
Description
This PR addresses missing build dependencies in the Windows bootstrap script. It adds automated installation of
protoc(viaprotobuf) andlibclang(via LLVM) usingwinget.These tools are hard dependencies for
prost-buildandbindgen, respectively. Without them, a freshcargo buildon Windows fails on crates likewarp_multi_agent_api. This change aligns the Windows bootstrap experience with the Linux improvements previously made in#9527.Linked Issue
Fixes #11301
ready-to-specorready-to-implement.Testing
Manual verification was performed by:
script/windows/bootstrap.ps1to ensure correctwingetIDs are used (protobufandLLVM.LLVM).Show-BootstrapPreviewcorrectly advertises the new installation steps.wingetsuccessfully identifies and installs these packages when they are missing.protoc --versionandclang --versionare available in thePATH../script/runScreenshots / Videos
N/A - scripting change for build environment setup.
Agent Mode