-
Notifications
You must be signed in to change notification settings - Fork 1
feat(build): add Buck2 as third build system for comparison #1036
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.
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
Conversation
Add Buck2 scaffolding alongside existing Gradle and Bazel builds for learning and comparing build systems in a real project. Includes: - Basic Buck2 configuration (.buckconfig, .buckversion, .buckroot) - Platform definitions using prelude platforms - CI workflow that builds and runs on Linux and macOS - Example targets (genrule, sh_binary, export_file) - Starlark lint job in CI (Buck2's equivalent to buildifier) Note: Builds require --target-platforms=prelude//platforms:default due to prelude's use of select() in rule definitions.
Summary of ChangesHello @albertocavalcante, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates Buck2 into the project as an experimental build system. The primary goal is to facilitate learning and comparison with existing Gradle and Bazel builds, without replacing them. It establishes the foundational configuration, introduces example build targets, and sets up necessary toolchains and platform definitions, laying the groundwork for further exploration of Buck2's capabilities within a real-world codebase. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds Buck2 configuration, CI workflow, example Buck targets, platform/toolchain placeholders, and repository ignores to enable Buck2-based builds and linting alongside existing build systems. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub
participant Runner as Runner
participant Buck2 as Buck2
participant Prelude as Prelude Repo
participant Targets as Build Targets
GH->>Runner: Trigger CI (push / PR / dispatch)
rect rgba(100,150,200,0.5)
Note over Runner: Lint Job
Runner->>Buck2: Download & install pinned Buck2
Runner->>Prelude: Clone/init prelude if missing
Runner->>Buck2: Discover BUCK files (excl. prelude)
Runner->>Buck2: Run starlark lint on files
end
rect rgba(150,100,200,0.5)
Note over Runner: Build Job (matrix)
Runner->>Buck2: Download & install Buck2 for matrix target
Runner->>Prelude: Ensure prelude initialized
Runner->>Buck2: Audit cells & list targets
Runner->>Buck2: Build targets with --target-platforms
Runner->>Targets: Run example target for verification
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces Buck2 as a new build system for comparison purposes, which is a great initiative for exploring different build tools. The scaffolding is well-structured with clear configuration files and examples. I've identified a couple of areas for improvement to enhance reproducibility and user experience. My main suggestions are to pin the Buck2 version for more stable CI builds and to update the example commands in the README to include a required flag, ensuring they work as expected out of the box.
- Pin Buck2 version to 2026-01-19 for reproducible builds - Add --target-platforms flag to README example commands - Update CI workflow to use pinned version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds Buck2 as a third build system alongside existing Gradle and Bazel builds for learning and comparing build systems. The addition is scaffolding-level, focusing on basic configuration and example targets rather than building the actual Groovy LSP project.
Changes:
- Added Buck2 configuration files (
.buckconfig,.buckversion,.buckroot) - Created example targets demonstrating genrule, sh_binary, and export_file rules
- Implemented CI workflow with linting and cross-platform builds
- Set up toolchain and platform definitions using Buck2 prelude
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
.buckconfig |
Main Buck2 configuration defining cells, platforms, and build settings |
.buckversion |
Specifies Buck2 version (currently "latest") |
.buckroot |
Empty marker file indicating Buck2 workspace root |
BUCK |
Root build file with documentation about Buck2 setup |
.gitignore |
Added Buck2-specific ignore patterns (buck-out/, prelude/, etc.) |
examples/hello/BUCK |
Example build targets demonstrating Buck2 rules |
examples/hello/greet.sh |
Shell script for sh_binary example |
examples/hello/README.md |
Documentation for example targets |
toolchains/BUCK |
Toolchain definitions using prelude's system toolchains |
platforms/BUCK |
Reserved for future custom platform definitions |
third_party/buck2/BUCK |
Reserved for external dependencies |
.github/workflows/ci-buck2.yml |
CI workflow for building and linting Buck2 files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PR Review: Buck2 Build System IntegrationSummaryThis PR adds Buck2 build system scaffolding alongside existing Gradle and Bazel builds. The implementation is well-structured and follows best practices for configuration files, CI setup, and documentation. ✅ Strengths1. Code Quality
2. Security
3. Performance
4. Git Workflow
|
| Criterion | Status | Notes |
|---|---|---|
| Code Quality | ✅ PASS | Well-documented, clear structure |
| Kotlin Conventions | N/A | No Kotlin code in this PR |
| Null Safety | N/A | No Kotlin code in this PR |
| Error Handling | ✅ PASS | Shell scripts use proper error handling |
| Security | ✅ PASS | No secrets, safe operations, SHA-pinned actions |
| TDD | ❌ FAIL | No tests for shell script logic |
| Coverage | Manual testing done, but incomplete checklist | |
| Edge Cases | No tests to verify edge case handling | |
| Performance | ✅ PASS | Efficient CI configuration |
| Git Workflow | ✅ PASS | Conventional commits, explicit file additions |
| Lint | ✅ PASS | Starlark lint passes |
🎯 Action Items
Before Merge:
-
[REQUIRED] Add tests for
examples/hello/greet.sh:// Example test structure (adapt to your test framework) @Test fun `greet script outputs default greeting without arguments`() @Test fun `greet script outputs custom greeting with argument`()
-
[REQUIRED] Update test plan checklist - check off the CI item (it's passing)
Nice to Have:
3. Document known limitations of buck2-prelude rules (which rules don't work due to FB-internal references)
4. Consider adding integration tests that verify Buck2 builds produce correct artifacts
🏆 Overall Assessment
This is a well-implemented scaffolding PR with excellent documentation and security practices. The main blocker is the absence of tests for shell script logic, which violates the TDD requirement in AGENTS.md.
Recommendation: Add tests for greet.sh before merging, then this will be ready to ship.
Review completed following .agent/rules/code-quality.md and .agent/rules/git-workflow.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@examples/hello/README.md`:
- Around line 13-23: Update the usage examples in the README examples for the
hello package to include the required
--target-platforms=prelude//platforms:default flag; specifically add this flag
to the three invocations shown (the buck2 build //examples/hello/... line, both
buck2 run //examples/hello:greet lines, and the buck2 build
//examples/hello:hello --show-output line) so that all example commands invoke
the same platform selection used by the prelude genrule.
🧹 Nitpick comments (4)
.buckversion (1)
1-4: Pin Buck2 to a date-tagged release instead oflatestfor reproducible builds.The
latesttag moves on every Buck2 push and creates nondeterministic builds across CI runs. Use a specific date-tagged release (format:YYYY-MM-DD, e.g.,2026-01-19) to ensure all developers and CI agents use the same Buck2 version..github/workflows/ci-buck2.yml (3)
92-100: Add error handling for empty file list in lint step.If no BUCK files are found (e.g., all deleted),
xargswith an empty input will still invokebuck2 starlark lintwith no arguments, which may produce unexpected behavior. Consider usingxargs --no-run-if-empty(GNU) or checking the file count.♻️ Suggested improvement
- name: Lint BUCK files run: | # Find all BUCK files (excluding prelude which is external) # Use xargs to properly handle the file list echo "Finding BUCK files to lint..." find . -name 'BUCK' -not -path './prelude/*' | sort | tee /tmp/buck_files.txt echo "" + if [ ! -s /tmp/buck_files.txt ]; then + echo "No BUCK files found to lint" + exit 0 + fi echo "Running starlark lint..." - xargs buck2 starlark lint < /tmp/buck_files.txt + xargs --no-run-if-empty buck2 starlark lint < /tmp/buck_files.txt
126-130: JDK setup may be unnecessary for current examples.The current Buck2 examples (
genrule,sh_binary,export_file) don't require Java. Consider removing the JDK setup step until Java-based Buck2 targets are added, to reduce CI time and dependencies. Alternatively, add a comment explaining the intent if this is preparation for future Java targets.
161-168: Consider extracting prelude initialization to a reusable action.The prelude initialization logic is duplicated between the
lintandbuildjobs. For maintainability, consider extracting this into a composite action or a reusable workflow step. This is a minor refactor that can be deferred.
- Pin prelude to commit 995098b for reproducible builds - Update actions/checkout to v6.0.1 (matching other workflows) - Update actions/setup-java to v5.1.0 (matching other workflows) - Fix typo: kotlincd_toolchain -> kotlin_toolchain
|
RE: macOS runner version (macos-14 vs macos-15) macos-14 is intentional: it provides Apple Silicon (aarch64) runners which match the The Buck2 build matrix is:
This is consistent with how the runner architecture maps to the target triple. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Cache prelude directory keyed by BUCK2_PRELUDE_COMMIT - Add TODO comment about dtolnay/install-buck2 action for future exploration - Closes #1038
Summary
Add Buck2 scaffolding alongside existing Gradle and Bazel builds for learning and comparing build systems in a real project.
.buckconfig,.buckversion,.buckroot)Background
This is a learning/comparison project to explore build system limitations, pros/cons, and benchmarks in a real-life codebase. Gradle remains the "official" build system.
Notes
--target-platforms=prelude//platforms:defaultflag due to prelude's use ofselect()in rule definitionsbuildifier, butbuck2 starlark lintprovides lintingTest plan
buck2 build //...succeeds locallybuck2 run //examples/hello:greetoutputs expected messagebuck2 starlark lintpasses on all BUCK filesSummary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.