-
Notifications
You must be signed in to change notification settings - Fork 583
fix: ci-barretenberg-full mode fixes #19466
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
fix: ci-barretenberg-full mode fixes #19466
Conversation
7b2e604 to
a7d9cd9
Compare
2cfc72b to
9fc3aef
Compare
ludamad
left a 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.
See above
## Summary - The vm2 library is only present when barretenberg is built with AVM enabled - When AVM is disabled (e.g., in some CI modes), libbarretenberg contains vm2_stub instead - This change makes build.rs check if libvm2.a exists before linking it ## Problem barretenberg-rs build was failing in CI with: ``` error: could not find native static library `vm2`, perhaps an -L flag is missing? ``` This happened because the cpp build was done without AVM enabled, so libvm2.a wasn't present. Co-authored-by: Jonathan Hao <jonathan@aztec-labs.com>
Addresses review feedback: - Add static library version of vm2_stub for external consumers - Always link vm2_stub instead of conditionally linking vm2 - This avoids caching issues and provides consistent behavior - vm2_stub throws at runtime if AVM recursion is attempted
710539f to
512a859
Compare
|
Will ask for review again once ci-barretenberg-full passes. |
|
Re: why linking barretenberg.a isn't enough: From # For this library we include everything but the env and wasi modules, as it is the responsibility of the
# consumer of this library to define how and in what environment its artifact will run.
So the current linking is:
Creating a single |
|
And why not cmake changes? |
1c8d0da to
4e7fe0a
Compare
Create a single static library that bundles everything barretenberg-rs needs: - All objects from libbarretenberg.a - env module (logstr, throw_or_abort_impl, env_hardware_concurrency) - vm2_stub (provides stub for create_avm2_recursion_constraints_goblin) This simplifies linking in barretenberg-rs to just one library and removes the need for --allow-multiple-definition and --start-group/--end-group.
4e7fe0a to
1afc054
Compare
You're right. I'm not sure what I was thinking. Done. |
|
LGTM now :) |
3854d2f
into
merge-train/barretenberg
BEGIN_COMMIT_OVERRIDE chore: Improve Chonk debug info (#19538) chore: translator non-native and decomp relations audit (#19081) chore: add safety to derive_generators and tweak pedersen scope (#19525) fix: ci-barretenberg-full mode fixes (#19466) test: use WASM backend for bbjs-test acir tests (#19529) fix: use absolute path in run_test.sh for CI fix: use env_objects in bb-external library fix: completeness issue in cycle scalar constructor from bigfield (#19475) chore: review a few minor files for ultra/mega audit (#19513) END_COMMIT_OVERRIDE
## Summary
Multiple fixes for `ci-barretenberg-full` mode:
1. **Link vm2_stub in barretenberg-rs** - Add static library version of
vm2_stub and always link it instead of conditionally linking vm2
2. **Propagate AVM env var** - Pass `AVM` environment variable to
isolated docker containers so tests use the correct `bb` binary
3. **Denoise barretenberg-rs** - Add `denoise` to build and test
commands for cleaner CI output
## Problem
`ci-barretenberg-full` mode was failing with multiple issues:
1. barretenberg-rs build failed with:
```
error: could not find native static library `vm2`, perhaps an -L flag is
missing?
```
This happened because `ci-barretenberg-full` builds with `AVM=0`, so
libvm2.a is not produced.
2. acir_tests failed with:
```
barretenberg/cpp/build/bin/bb-avm: No such file or directory
```
The `AVM` environment variable wasn't being passed to isolated docker
containers, causing `find-bb` to incorrectly select `bb-avm` instead of
`bb`.
## Solution
1. Create a static library `libvm2_stub.a` from the existing vm2_stub
OBJECT library and always link it in barretenberg-rs (the stub throws at
runtime if AVM recursion is attempted)
2. Propagate `AVM` env var to isolated docker containers in
`ci3/docker_isolate`
3. Use `denoise` for barretenberg-rs build and test commands
## Caching
Verified that `libvm2_stub.a` will be included in the cache upload since
it's built to `build/lib/` and the native build cache uploads
`build/{bin,lib}`.
---------
Co-authored-by: johnathan79717 <511655+johnathan79717@users.noreply.github.com>
Summary
Multiple fixes for
ci-barretenberg-fullmode:AVMenvironment variable to isolated docker containers so tests use the correctbbbinarydenoiseto build and test commands for cleaner CI outputProblem
ci-barretenberg-fullmode was failing with multiple issues:barretenberg-rs build failed with:
This happened because
ci-barretenberg-fullbuilds withAVM=0, so libvm2.a is not produced.acir_tests failed with:
The
AVMenvironment variable wasn't being passed to isolated docker containers, causingfind-bbto incorrectly selectbb-avminstead ofbb.Solution
Create
libbb-external.athat includes everything barretenberg-rs needs:This simplifies linking in barretenberg-rs to just one library and removes the need for
--allow-multiple-definitionand--start-group/--end-group.Propagate
AVMenv var to isolated docker containers inci3/docker_isolateUse
denoisefor barretenberg-rs build and test commandsCaching
libbb-external.ais built tobuild/lib/and the native build cache uploadsbuild/{bin,lib}, so it will be cached automatically.