Skip to content
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.

Sign up for GitHub

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

Jump to bottom

chore: Move barretenberg to top of repo. Make circuits build off barretenberg build. #2221

Merged
merged 37 commits into from
Sep 13, 2023
Merged

chore: Move barretenberg to top of repo. Make circuits build off barretenberg build. #2221

merged 37 commits into from
Sep 13, 2023

Conversation

charlielye
Copy link
Contributor

@charlielye charlielye commented Sep 11, 2023 •
edited
Loading

This moves barretenberg out of circuits project, to the top level of the repo, and cleans up a lot of build-system stuff.

BB and Circuits

  • Circuits expects barretenberg to still be where it was before (root of its repo), but it has a symlink now to the top level.
  • Docker scoping is handled by ingesting barretenberg from prior build, which outputs both the source code, and libraries needed for circuits (libbarretenberg, libenv, libwasi).
  • Circuits not longer builds barretenberg itself.
  • barretenberg now does a formatting check over it's code as part of build. ./format.sh check
  • Remove some refs to openmp in containers as we're phasing it out....
  • Remove some cmake around installing barretenberg, as this isn't needed anymore.

Build System

  • Remove hacks around e2e running locally. Use now official cond_run_script script.
  • Remove superfluous scripts and normalise around the cond_spot_run_script and remote_runner script.
  • cond_spot_run_build and cond_spot_run_test both implemented in terms of above.
  • Now test runs are handled by remote_runner they perform a full checkout of code and fit in the general pattern of the build system. No more hacky arbitrary script folder to remote copies.
  • Added yarn-project-base as dep in build-manifest.json to all TS projects so rebuilds are correctly computed.
  • Improved performance of dependencies command in query_manifest and also rebuildPatterns.
  • query_manifest outputs absolute paths by default, meaning can remove various assumptions in other scripts as to current working directory.

@ludamad
Copy link
Collaborator

ludamad commented Sep 11, 2023

fyi there's lots of conflicts but that's just Palla (Santiago) doing the format so you can probably just redo that

ludamad
ludamad reviewed Sep 13, 2023
View reviewed changes
barretenberg/cpp/format.sh
@@ -1,5 +1,5 @@
#!/bin/bash
set -eu
set -e
Copy link
Collaborator

@ludamad ludamad Sep 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'd prefer to just change the $1 to ${1:-} but not a big deal

ludamad
ludamad reviewed Sep 13, 2023
View reviewed changes
circuits/cpp/CMakePresets.json
@@ -139,8 +123,7 @@
"inherits": "default",
"binaryDir": "build-fuzzing",
"cacheVariables": {
"FUZZING": "ON",
"CMAKE_BBERG_PRESET": "fuzzing"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CMAKE_BBERG_PRESET no longer neded?

ludamad
ludamad approved these changes Sep 13, 2023
View reviewed changes
Copy link
Collaborator

@ludamad ludamad left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with a merge, but there's a lot here. If stuff goes wrong it'll be a bit hard for people to go back to this PR as it struggles to load. It'd be nice to split the concerns in the future

Good though to move forward and see how things are.

@ludamad
Copy link
Collaborator

ludamad commented Sep 13, 2023

Looks like vscode follows git blame, which is I think the most high profile git history use case, so that's working nicely

@charlielye charlielye merged commit 404ec34 into master Sep 13, 2023
2 checks passed
@charlielye charlielye deleted the cl/move_bb branch September 13, 2023 14:16
@AztecBot AztecBot mentioned this pull request Sep 13, 2023
Merged
dbanks12
dbanks12 reviewed Sep 13, 2023
View reviewed changes
Copy link
Contributor

@dbanks12 dbanks12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Late on this, but LGTM!

PhilWindle pushed a commit that referenced this pull request Sep 14, 2023
@AztecBot
chore(master): Release 0.7.1 (#2268)
4d3e341 
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-packages: 0.7.1</summary>

##
[0.7.1](aztec-packages-v0.7.0...aztec-packages-v0.7.1)
(2023-09-14)


### Features

* Build system handles dynamic deps first class.
([#2283](#2283))
([f66077a](f66077a))
* Build_manifest default tweaks.
([#2287](#2287))
([c8a5cfb](c8a5cfb))
* **build:** Build multi-architecture docker images for aztec-sandbox
([#2305](#2305))
([8ee61b8](8ee61b8))
* Cli "unbox" command
([#2029](#2029))
([26ab88f](26ab88f))
* Creating an SMT verification module
([#1932](#1932))
([4642b61](4642b61))
* Token standard
([#2069](#2069))
([5e8fbf2](5e8fbf2))


### Bug Fixes

* Ensure_note_hash_exists
([#2256](#2256))
([271b060](271b060))
* Msgpack stack blowups on schema gen
([#2259](#2259))
([1afc566](1afc566))
* Noir bootstrap
([#2274](#2274))
([f85db49](f85db49))
* Workaround sequencer timeout
([#2269](#2269))
([9fc3f3d](9fc3f3d))


### Miscellaneous

* Bump nargo to 0.11.1-aztec.0
([#2298](#2298))
([8b76a12](8b76a12))
* **ci:** Mirror Aztec-nr
([#2270](#2270))
([c57f027](c57f027))
* **circuits:** Base rollup cbind msgpack
([#2263](#2263))
([0d4c707](0d4c707))
* **circuits:** Clean up of some superfluous header includes
([#2302](#2302))
([5e53345](5e53345))
* **circuits:** Removing assertMemberLength on Tuple objects
([#2296](#2296))
([0247b85](0247b85))
* Consolidate mirror repos on a nightly schedule
([#1994](#1994))
([1a586c4](1a586c4))
* **docs:** Rename to aztec.nr
([#1943](#1943))
([a91db48](a91db48))
* Move barretenberg to top of repo. Make circuits build off barretenberg
build.
([#2221](#2221))
([404ec34](404ec34))
* Replace native token in lending contract
([#2276](#2276))
([c46b3c8](c46b3c8))
* **subrepo:** Push aztec-nr, update default branches
([#2300](#2300))
([80c9b77](80c9b77))
* Updated `acvm_js`
([#2272](#2272))
([9f1a3a5](9f1a3a5))
</details>

<details><summary>barretenberg.js: 0.7.1</summary>

##
[0.7.1](barretenberg.js-v0.7.0...barretenberg.js-v0.7.1)
(2023-09-14)


### Miscellaneous

* Move barretenberg to top of repo. Make circuits build off barretenberg
build.
([#2221](#2221))
([404ec34](404ec34))
</details>

<details><summary>barretenberg: 0.7.1</summary>

##
[0.7.1](barretenberg-v0.7.0...barretenberg-v0.7.1)
(2023-09-14)


### Miscellaneous

* Move barretenberg to top of repo. Make circuits build off barretenberg
build.
([#2221](#2221))
([404ec34](404ec34))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbanks12 dbanks12 dbanks12 left review comments

@ludamad ludamad ludamad approved these changes

@spalladino spalladino Awaiting requested review from spalladino

Assignees
No one assigned
Labels
None yet
Projects
A3
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@charlielye @ludamad @dbanks12